Bug 232404

Summary: [Tools] Enable -Wformat=2 warnings
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch v1
darin: review+
Patch for landing none

Description David Kilzer (:ddkilzer) 2021-10-27 14:33:27 PDT
Enable -Wformat=2 warnings on projects in the Tools folder.

In practice, only TestWebKitAPI required any changes after adding this warning flag.
Comment 1 Radar WebKit Bug Importer 2021-10-27 14:33:48 PDT
<rdar://problem/84728058>
Comment 2 David Kilzer (:ddkilzer) 2021-10-27 15:26:25 PDT
Created attachment 442638 [details]
Patch v1
Comment 3 Darin Adler 2021-10-27 15:34:59 PDT
Comment on attachment 442638 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=442638&action=review

> Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadAlternateHTMLString.mm:174
> +    NSString *const html = @"<script>var xhr = new XMLHttpRequest(); xhr.open('GET', 'http://127.0.0.1:%d/', true); xhr.send();</script>";

I suggest you use constexpr instead. One nice thing about that is that then it can be at the beginning of the line.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/OpenAndCloseWindow.mm:239
> +    NSString *const windowOpenFormatString = @"window.open(\"about:blank\", \"_blank\", \"%@\")";

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h:83
> +            NSString *const format = @"HTTP/1.1 200 OK\r\n"

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h:95
> +    NSURLRequest *requestWithURLString(NSString *url) { return [NSURLRequest requestWithURL:[NSURL URLWithString:url]]; }

I would name this argument "string", not "url".

> Tools/TestWebKitAPI/cocoa/HTTPServer.mm:368
> +    return [NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"%s://127.0.0.1:%d%s", scheme, port(), path.utf8().data()]]];

No good reason to convert to a UTF8 string just to format. Can use %@ and use path.createCFString().get() instead.
Comment 4 David Kilzer (:ddkilzer) 2021-10-27 16:58:21 PDT
Comment on attachment 442638 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=442638&action=review

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h:95
>> +    NSURLRequest *requestWithURLString(NSString *url) { return [NSURLRequest requestWithURL:[NSURL URLWithString:url]]; }
> 
> I would name this argument "string", not "url".

Changed `url` to `urlString`, and will make the other changes as suggested. Thanks!
Comment 5 David Kilzer (:ddkilzer) 2021-10-27 16:59:35 PDT
Created attachment 442652 [details]
Patch for landing
Comment 6 EWS 2021-10-28 08:12:15 PDT
Committed r284977 (243625@main): <https://commits.webkit.org/243625@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442652 [details].