Format strings: offer a fixit for Darwin's %D/%U/%O to ISO %d/%u/%o.

<rdar://problem/12061922>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@163772 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/Analyses/FormatString.h

@@ -162,7 +162,7 @@ public:
     ScanfConvBeg = ScanListArg, ScanfConvEnd = ScanListArg
   };
 
-  ConversionSpecifier(bool isPrintf)
+  ConversionSpecifier(bool isPrintf = true)
     : IsPrintf(isPrintf), Position(0), EndScanList(0), kind(InvalidSpecifier) {}
 
   ConversionSpecifier(bool isPrintf, const char *pos, Kind k)
@@ -200,6 +200,8 @@ public:
   const char *toString() const;
 
   bool isPrintfKind() const { return IsPrintf; }
+  
+  llvm::Optional<ConversionSpecifier> getStandardSpecifier() const;
 
 protected:
   bool IsPrintf;

lib/Analysis/FormatString.cpp

@@ -527,6 +527,29 @@ const char *ConversionSpecifier::toString() const {
   return NULL;
 }
 
+llvm::Optional<ConversionSpecifier>
+ConversionSpecifier::getStandardSpecifier() const {
+  ConversionSpecifier::Kind NewKind;
+  
+  switch (getKind()) {
+  default:
+    return llvm::Optional<ConversionSpecifier>();
+  case DArg:
+    NewKind = dArg;
+    break;
+  case UArg:
+    NewKind = uArg;
+    break;
+  case OArg:
+    NewKind = oArg;
+    break;
+  }
+
+  ConversionSpecifier FixedCS(*this);
+  FixedCS.setKind(NewKind);
+  return FixedCS;
+}
+
 //===----------------------------------------------------------------------===//
 // Methods on OptionalAmount.
 //===----------------------------------------------------------------------===//

lib/Sema/SemaChecking.cpp

@@ -2095,11 +2095,28 @@ void CheckFormatHandler::HandleNonStandardLengthModifier(
 void CheckFormatHandler::HandleNonStandardConversionSpecifier(
     const analyze_format_string::ConversionSpecifier &CS,
     const char *startSpecifier, unsigned specifierLen) {
-  EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard) << CS.toString()
-                       << 1,
-                       getLocationOfByte(CS.getStart()),
-                       /*IsStringLocation*/true,
-                       getSpecifierRange(startSpecifier, specifierLen));
+  using namespace analyze_format_string;
+
+  // See if we know how to fix this conversion specifier.
+  llvm::Optional<ConversionSpecifier> FixedCS = CS.getStandardSpecifier();
+  if (FixedCS) {
+    EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard)
+                          << CS.toString() << /*conversion specifier*/1,
+                         getLocationOfByte(CS.getStart()),
+                         /*IsStringLocation*/true,
+                         getSpecifierRange(startSpecifier, specifierLen));
+
+    CharSourceRange CSRange = getSpecifierRange(CS.getStart(), CS.getLength());
+    S.Diag(getLocationOfByte(CS.getStart()), diag::note_format_fix_specifier)
+      << FixedCS->toString()
+      << FixItHint::CreateReplacement(CSRange, FixedCS->toString());
+  } else {
+    EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard)
+                          << CS.toString() << /*conversion specifier*/1,
+                         getLocationOfByte(CS.getStart()),
+                         /*IsStringLocation*/true,
+                         getSpecifierRange(startSpecifier, specifierLen));
+  }
 }
 
 void CheckFormatHandler::HandlePosition(const char *startPos,

test/FixIt/format-darwin.m

@@ -1,11 +1,11 @@
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -fblocks -verify %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -fblocks -Wformat-non-iso -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fblocks -Wformat-non-iso -verify %s
 
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck %s
 
-// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK-32 %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK-64 %s
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck -check-prefix=CHECK-32 %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks -Wformat-non-iso %s 2>&1 | FileCheck -check-prefix=CHECK-64 %s
 
 int printf(const char * restrict, ...);
 
@@ -181,11 +181,18 @@ void testCasts() {
 }
 
 void testCapitals() {
-  printf("%D", 1); // no-warning
-  printf("%U", 1); // no-warning
-  printf("%O", 1); // no-warning
+  printf("%D", 1); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}}
+  printf("%U", 1); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'u'?}}
+  printf("%O", 1); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'o'?}}
   
-  printf("%lD", 1); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{184:12-184:13}:"d"
+  // CHECK: fix-it:"{{.*}}":{185:12-185:13}:"u"
+  // CHECK: fix-it:"{{.*}}":{186:12-186:13}:"o"
 
-  // CHECK: fix-it:"{{.*}}":{188:11-188:14}:"%D"
+  
+  printf("%lD", 1); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}} expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+
+  // FIXME: offering two somewhat-conflicting fixits is less than ideal.
+  // CHECK: fix-it:"{{.*}}":{193:13-193:14}:"d"
+  // CHECK: fix-it:"{{.*}}":{193:11-193:14}:"%D"
 }

test/Sema/format-strings-darwin.c

@@ -22,12 +22,12 @@ void test() {
   printf("%O", tooLong);
 
 #ifdef ALLOWED
-  // expected-warning@-8 {{'D' conversion specifier is not supported by ISO C}}
-  // expected-warning@-8 {{'D' conversion specifier is not supported by ISO C}} expected-warning@-8 {{format specifies type 'int' but the argument has type 'long'}}
-  // expected-warning@-8 {{'U' conversion specifier is not supported by ISO C}}
-  // expected-warning@-8 {{'U' conversion specifier is not supported by ISO C}} expected-warning@-8 {{format specifies type 'unsigned int' but the argument has type 'long'}}
-  // expected-warning@-8 {{'O' conversion specifier is not supported by ISO C}}
-  // expected-warning@-8 {{'O' conversion specifier is not supported by ISO C}} expected-warning@-8 {{format specifies type 'unsigned int' but the argument has type 'long'}}
+  // expected-warning@-8 {{'D' conversion specifier is not supported by ISO C}} expected-note@-8 {{did you mean to use 'd'?}}
+  // expected-warning@-8 {{'D' conversion specifier is not supported by ISO C}} expected-note@-8 {{did you mean to use 'd'?}} expected-warning@-8 {{format specifies type 'int' but the argument has type 'long'}}
+  // expected-warning@-8 {{'U' conversion specifier is not supported by ISO C}} expected-note@-8 {{did you mean to use 'u'?}}
+  // expected-warning@-8 {{'U' conversion specifier is not supported by ISO C}} expected-note@-8 {{did you mean to use 'u'?}} expected-warning@-8 {{format specifies type 'unsigned int' but the argument has type 'long'}}
+  // expected-warning@-8 {{'O' conversion specifier is not supported by ISO C}} expected-note@-8 {{did you mean to use 'o'?}}
+  // expected-warning@-8 {{'O' conversion specifier is not supported by ISO C}} expected-note@-8 {{did you mean to use 'o'?}} expected-warning@-8 {{format specifies type 'unsigned int' but the argument has type 'long'}}
 #else
   // expected-warning@-15 {{invalid conversion specifier 'D'}}
   // expected-warning@-15 {{invalid conversion specifier 'D'}}
@@ -40,25 +40,25 @@ void test() {
 
 #ifdef ALLOWED
 void testPrintf(short x, long y) {
-  printf("%hD", x); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("%lD", y); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("%hU", x); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("%lU", y); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("%hO", x); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("%lO", y); // expected-warning{{conversion specifier is not supported by ISO C}}
+  printf("%hD", x); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}}
+  printf("%lD", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}}
+  printf("%hU", x); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'u'?}}
+  printf("%lU", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'u'?}}
+  printf("%hO", x); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'o'?}}
+  printf("%lO", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'o'?}}
 
-  printf("%+'0.5lD", y); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("% '0.5lD", y); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("%#0.5lO", y); // expected-warning{{conversion specifier is not supported by ISO C}}
-  printf("%'0.5lU", y); // expected-warning{{conversion specifier is not supported by ISO C}}
+  printf("%+'0.5lD", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}}
+  printf("% '0.5lD", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}}
+  printf("%#0.5lO", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'o'?}}
+  printf("%'0.5lU", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'u'?}}
 }
 
 void testScanf(short *x, long *y) {
-  scanf("%hD", x); // expected-warning{{conversion specifier is not supported by ISO C}}
-  scanf("%lD", y); // expected-warning{{conversion specifier is not supported by ISO C}}
-  scanf("%hU", x); // expected-warning{{conversion specifier is not supported by ISO C}}
-  scanf("%lU", y); // expected-warning{{conversion specifier is not supported by ISO C}}
-  scanf("%hO", x); // expected-warning{{conversion specifier is not supported by ISO C}}
-  scanf("%lO", y); // expected-warning{{conversion specifier is not supported by ISO C}}
+  scanf("%hD", x); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}}
+  scanf("%lD", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'd'?}}
+  scanf("%hU", x); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'u'?}}
+  scanf("%lU", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'u'?}}
+  scanf("%hO", x); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'o'?}}
+  scanf("%lO", y); // expected-warning{{conversion specifier is not supported by ISO C}} expected-note {{did you mean to use 'o'?}}
 }
 #endif