Merge pull request #68881 from apple/prefix-smashing

Diagnose possible enum common-prefix mistakes

Author: swift-ci

include/swift/AST/DiagnosticsSema.def

@@ -104,6 +104,12 @@ ERROR(could_not_find_enum_case,none,
       "enum type %0 has no case %1; did you mean %2?",
       (Type, DeclNameRef, DeclName))
 
+ERROR(could_not_find_imported_enum_case,none,
+      "type %0 has no case %1, but it does have a case named %2; if %1 worked "
+      "before, a recent change to the underlying C enum may have affected how "
+      "prefixes are stripped from its case names",
+      (Type, DeclNameRef, ValueDecl *))
+
 NOTE(did_you_mean_raw_type,none,
      "did you mean to specify a raw type on the enum declaration?", ())
 
@@ -1728,6 +1734,9 @@ NOTE(fixit_add_private_for_objc_implementation,none,
 NOTE(fixit_add_final_for_objc_implementation,none,
      "add 'final' to define a Swift %0 that cannot be overridden",
      (DescriptiveDeclKind))
+NOTE(fixit_add_nonobjc_for_objc_implementation,none,
+     "add '@nonobjc' to define a Swift-only %0",
+     (DescriptiveDeclKind))
 
 ERROR(objc_implementation_wrong_category,none,
       "%kind0 should be implemented in extension for "

include/swift/Basic/StringExtras.h

@@ -223,6 +223,8 @@ namespace swift {
 
       reverse_iterator rbegin() const { return reverse_iterator(end()); }
       reverse_iterator rend() const { return reverse_iterator(begin()); }
+
+      bool hasWordStartingAt(unsigned targetPosition) const;
     };
 
     /// Retrieve the camelCase words in the given string.
@@ -236,6 +238,10 @@ namespace swift {
     /// case of the first letter.
     bool startsWithIgnoreFirstCase(StringRef word1, StringRef word2);
 
+    /// Check whether the first word ends with the second word, ignoring the
+    /// case of the first word (handles initialisms).
+    bool hasWordSuffix(StringRef haystack, StringRef needle);
+
     /// Lowercase the first word within the given camelCase string.
     ///
     /// \param string The string to lowercase.

lib/Basic/StringExtras.cpp

@@ -215,6 +215,17 @@ void WordIterator::computePrevPosition() const {
   PrevPositionValid = true;
 }
 
+bool camel_case::Words::hasWordStartingAt(unsigned targetPosition) const {
+  // Iterate over the words until we see one at or past targetPosition.
+  // FIXME: Is there a faster way to do this by looking at the characters around
+  //        the position?
+  for (auto i = begin(); i != end() && i.getPosition() <= targetPosition; i++) {
+    if (i.getPosition() == targetPosition)
+      return true;
+  }
+  return false;
+}
+
 StringRef camel_case::getFirstWord(StringRef string) {
   if (string.empty())
     return "";
@@ -249,6 +260,30 @@ bool camel_case::startsWithIgnoreFirstCase(StringRef word1, StringRef word2) {
   return word1.substr(1, word2.size() - 1) == word2.substr(1);
 }
 
+bool camel_case::hasWordSuffix(StringRef haystack, StringRef needle) {
+  // Is it even possible for one to be a suffix of the other?
+  if (needle.empty() || haystack.size() <= needle.size())
+    return false;
+
+  // Does haystack have a word boundary at the right position?
+  auto targetPosition = haystack.size() - needle.size();
+  if (!Words(haystack).hasWordStartingAt(targetPosition))
+    return false;
+
+  StringRef suffix = haystack.substr(targetPosition);
+
+  // Fast path: Without potentially copying the strings, do they match?
+  if (sameWordIgnoreFirstCase(suffix, needle))
+    return true;
+
+  // Flatten out leading initialisms. Do they match?
+  SmallString<32> suffixScratch, needleScratch;
+  auto suffixFlat = toLowercaseInitialisms(suffix, suffixScratch);
+  auto needleFlat = toLowercaseInitialisms(needle, needleScratch);
+
+  return suffixFlat == needleFlat;
+}
+
 StringRef camel_case::toLowercaseWord(StringRef string,
                                       SmallVectorImpl<char> &scratch) {
   if (string.empty())

lib/Sema/CSDiagnostics.cpp

@@ -4136,6 +4136,84 @@ DeclName MissingMemberFailure::findCorrectEnumCaseName(
   return (candidate ? candidate->getName() : DeclName());
 }
 
+/// If  \p instanceTy is an imported Clang enum, find the best imported case (if
+/// any) which has \p name as a (capitalization-adjusted) suffix.
+ValueDecl *MissingMemberFailure::
+findImportedCaseWithMatchingSuffix(Type instanceTy, DeclNameRef name) {
+  IterableDeclContext *idc = nullptr;
+
+  if (auto ED = instanceTy->getEnumOrBoundGenericEnum()) {
+    idc = ED;
+  }
+  else if (auto SD = instanceTy->getStructOrBoundGenericStruct()) {
+    // Did ClangImporter add OptionSet to this struct?
+    for (auto protoAttr :
+            SD->getAttrs().getAttributes<SynthesizedProtocolAttr>()) {
+      auto knownProto = protoAttr->getProtocol()->getKnownProtocolKind();
+      if (knownProto == KnownProtocolKind::OptionSet) {
+        idc = SD;
+        break;
+      }
+    }
+  }
+
+  if (!idc || !idc->getDecl()->hasClangNode())
+    // Not an imported clang enum.
+    return nullptr;
+
+  auto betterMatch = [](ValueDecl *a, ValueDecl *b) -> ValueDecl * {
+#define WORSE(BAD_CONDITION) do {     \
+      auto aBad = a BAD_CONDITION;    \
+      auto bBad = b BAD_CONDITION;    \
+      if (aBad > bBad)                \
+        return b;                     \
+      if (aBad < bBad)                \
+        return a;                     \
+    } while (false)
+
+    // Is one null? Return the other.
+    WORSE(== nullptr);
+
+    assert((a && b) && "neither should be null here");
+    ASTContext &ctx = a->getASTContext();
+
+    // Is one more available than the other?
+    WORSE(->getAttrs().isUnavailable(ctx));
+    WORSE(->getAttrs().getDeprecated(ctx));
+
+    // Does one have a shorter name (so the non-matching prefix is shorter)?
+    WORSE(->getName().getBaseName().userFacingName().size());
+#undef WORSE
+
+    // Arbitrary tiebreaker: compare the names.
+    if (a->getName().compare(b->getName()) > 0)
+      return b;
+    return a;
+  };
+
+  StringRef needle = name.getBaseName().userFacingName();
+
+  ValueDecl *bestMatch = nullptr;
+  for (auto member : idc->getMembers()) {
+    auto VD = dyn_cast<ValueDecl>(member);
+    if (!VD)
+      continue;
+    if (!(isa<EnumElementDecl>(VD) || isa<VarDecl>(VD)))
+      continue;
+    if (VD->isInstanceMember())
+      continue;
+    if (!VD->getInterfaceType()->isEqual(instanceTy))
+      continue;
+    if (!camel_case::hasWordSuffix(VD->getName().getBaseName().userFacingName(),
+                                   needle))
+      continue;
+
+    // This member matches. Is it the best match?
+    bestMatch = betterMatch(bestMatch, VD);
+  }
+  return bestMatch;
+}
+
 bool MissingMemberFailure::diagnoseAsError() {
   auto anchor = getRawAnchor();
   auto memberBase = getAnchor();
@@ -4254,7 +4332,16 @@ bool MissingMemberFailure::diagnoseAsError() {
       } else {
         emitBasicError(baseType);
       }
-    } else {
+    } else if (ValueDecl *bestMatch =
+                  findImportedCaseWithMatchingSuffix(instanceTy, getName())) {
+       // Sometimes Clang Importer's case prefix stripping unexpectedly
+       // changes the name of unrelated cases when someone adds a new case or
+       // changes the deprecation of an existing case. If this is such an
+       // imported type and we can find a case which has `getName()` as a
+       // suffix, mention this possibility.
+       emitDiagnostic(diag::could_not_find_imported_enum_case, instanceTy,
+                      getName(), bestMatch);
+     } else {
       emitBasicError(baseType);
     }
   } else if (auto moduleTy = baseType->getAs<ModuleType>()) {

lib/Sema/CSDiagnostics.h

@@ -1276,6 +1276,9 @@ class MissingMemberFailure : public InvalidMemberRefFailure {
   static DeclName findCorrectEnumCaseName(Type Ty,
                                           TypoCorrectionResults &corrections,
                                           DeclNameRef memberName);
+
+  static ValueDecl *findImportedCaseWithMatchingSuffix(Type instanceTy,
+                                                       DeclNameRef name);
 };
 
 class UnintendedExtraGenericParamMemberFailure final

test/ClangImporter/enum.swift

@@ -146,6 +146,27 @@ _ = NSPrefixWordBreakReordered2Custom.problemCase == .goodCase
 _ = NSPrefixWordBreakReordered2Custom.problemCase == .PrefixWordBreakReordered2DeprecatedBadCase // expected-warning {{deprecated}}
 _ = NSPrefixWordBreakReordered2Custom.problemCase == .deprecatedGoodCase // expected-warning {{deprecated}}
 
+#if !IRGEN
+_ = NSPrefixWordBreakForgotToDeprecate.newName1 // expected-error {{type 'NSPrefixWordBreakForgotToDeprecate' has no case 'newName1', but it does have a case named 'PrefixWordBreakNewName1'; if 'newName1' worked before, a recent change to the underlying C enum may have affected how prefixes are stripped from its case names}}
+_ = NSPrefixWordBreakForgotToDeprecate.newName2 // expected-error {{type 'NSPrefixWordBreakForgotToDeprecate' has no case 'newName2', but it does have a case named 'PrefixWordBreakNewName2'; if 'newName2' worked before, a recent change to the underlying C enum may have affected how prefixes are stripped from its case names}}
+_ = NSPrefixWordBreakForgotToDeprecate.noMatches // expected-error {{type 'NSPrefixWordBreakForgotToDeprecate' has no member 'noMatches'}}
+
+_ = NSPrefixWordBreakInvalidWord.foo // expected-error {{type 'NSPrefixWordBreakInvalidWord' has no case 'foo', but it does have a case named 'BreakFoo'; if 'foo' worked before, a recent change to the underlying C enum may have affected how prefixes are stripped from its case names}}
+_ = NSPrefixWordBreakInvalidWord.bar // expected-error {{type 'NSPrefixWordBreakInvalidWord' has no case 'bar', but it does have a case named 'BreakBar'; if 'bar' worked before, a recent change to the underlying C enum may have affected how prefixes are stripped from its case names}}
+_ = NSPrefixWordBreakInvalidWord.noMatches // expected-error {{type 'NSPrefixWordBreakInvalidWord' has no member 'noMatches'}}
+
+_ = NSPrefixWordBreakSuffixTieBreakers.forTest1 // expected-error {{Better}}
+_ = NSPrefixWordBreakSuffixTieBreakers.forTest2 // expected-error {{Better}}
+_ = NSPrefixWordBreakSuffixTieBreakers.forTest3 // expected-error {{Better}}
+_ = NSPrefixWordBreakSuffixTieBreakers.forTest4 // expected-error {{Better}}
+_ = NSPrefixWordBreakSuffixTieBreakers.forTest5 // expected-error {{Better}}
+
+_ = [
+  .newOption1,  // expected-error {{type 'NSPrefixWordBreakOptions.ArrayLiteralElement' (aka 'NSPrefixWordBreakOptions') has no case 'newOption1', but it does have a case named 'prefixWordBreakNewOption1'; if 'newOption1' worked before, a recent change to the underlying C enum may have affected how prefixes are stripped from its case names}}
+  .newOption2,  // expected-error {{type 'NSPrefixWordBreakOptions.ArrayLiteralElement' (aka 'NSPrefixWordBreakOptions') has no case 'newOption2', but it does have a case named 'prefixWordBreakNewOption2'; if 'newOption2' worked before, a recent change to the underlying C enum may have affected how prefixes are stripped from its case names}}
+] as NSPrefixWordBreakOptions
+#endif
+
 _ = NSSwiftNameAllTheThings.Foo == .Bar
 _ = NSSwiftNameBad.`class`
 

test/Inputs/clang-importer-sdk/usr/include/Foundation.h

@@ -387,6 +387,47 @@ typedef NS_ENUM(NSInteger, NSPrefixWordBreakReordered2Custom) {
   NSPrefixWordBreakReordered2DeprecatedGoodCase __attribute__((deprecated)),
 };
 
+typedef NS_ENUM(NSInteger, NSPrefixWordBreakForgotToDeprecate) {
+  NSPrefixWordBreakNewName1,
+  NSPrefixWordBreakNewName2,
+  NSOldName1PrefixWordBreak = NSPrefixWordBreakNewName1, // should have been deprecated
+};
+
+typedef NS_ENUM(NSInteger, NSPrefixWordBreakInvalidWord) {
+  NSPrefixWordBreakFoo,
+  NSPrefixWordBreakBar,
+  NSPrefixWordBreak42,  // expected prefix would have left an invalid identifier
+};
+
+// Check tiebreaking rules. In each case, the diagnostic should choose the 'Better' case over the 'Worse' case.
+typedef NS_ENUM(NSInteger, NSPrefixWordBreakSuffixTieBreakers) {
+  // Available preferred over unavailable.
+  NSPrefixWordBreakBetterForTest1,
+  NSPrefixWordBreakWorseForTest1 __attribute__((unavailable)),
+
+  // Un-deprecated preferred over deprecated.
+  NSPrefixWordBreakBetterForTest2,
+  NSPrefixWordBreakWorseForTest2 __attribute__((deprecated)),
+
+  // Shorter preferred over longer (it's a closer match).
+  NSPrefixWordBreakBetterForTest3,
+  NSPrefixWordBreakWorseXXXForTest3,
+
+  // Alphabetically first preferred over second (arbitrary tiebreaker).
+  NSPrefixWordBreakBetterForTest4,
+  NSPrefixWordBreakWorseXForTest4,
+
+  // Make sure we're not choosing based on ordering.
+  NSPrefixWordBreakWorseXForTest5,
+  NSPrefixWordBreakBetterForTest5,
+};
+
+typedef NS_OPTIONS(NSInteger, NSPrefixWordBreakOptions) {
+  NSPrefixWordBreakNewOption1 = 0x1,
+  NSPrefixWordBreakNewOption2 = 0x2,
+  NSOldOption1PrefixWordBreak = NSPrefixWordBreakNewOption1, // should have been deprecated
+};
+
 typedef NS_ENUM(NSInteger, NSSwiftNameAllTheThings) {
   NSSwiftNameAllTheThingsA __attribute__((swift_name("Foo"))),
   NSSwiftNameAllTheThingsB __attribute__((swift_name("Bar"))),