Merge pull request swiftlang#25977 from roop/sr11037_fixit_platform_condition_kind

[Parser][QoI] Offer fixit for changing the platform condition kind

Author: beccadax

include/swift/AST/PlatformConditionKinds.def

@@ -0,0 +1,44 @@
+//===--- PlatformConditionKinds.def - Kinds of Platform Conditions - C++ ---*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines macros used for macro-metaprogramming with the kinds of
+// platform conditions that can be used for conditional compilation.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef PLATFORM_CONDITION
+#define PLATFORM_CONDITION(LABEL, IDENTIFIER)
+#endif
+#ifndef PLATFORM_CONDITION_
+#define PLATFORM_CONDITION_(LABEL, IDENTIFIER) PLATFORM_CONDITION(LABEL, "_" IDENTIFIER)
+#endif
+
+/// The active os target (OSX, iOS, Linux, etc.)
+PLATFORM_CONDITION(OS, "os")
+
+/// The active arch target (x86_64, i386, arm, arm64, etc.)
+PLATFORM_CONDITION(Arch, "arch")
+
+/// The active endianness target (big or little)
+PLATFORM_CONDITION_(Endianness, "endian")
+
+/// Runtime support (_ObjC or _Native)
+PLATFORM_CONDITION_(Runtime, "runtime")
+
+/// Conditional import of module
+PLATFORM_CONDITION(CanImport, "canImport")
+
+/// Target Environment (currently just 'simulator' or absent)
+PLATFORM_CONDITION(TargetEnvironment, "targetEnvironment")
+
+#undef PLATFORM_CONDITION
+#undef PLATFORM_CONDITION_

include/swift/Basic/LangOptions.h

@@ -37,18 +37,8 @@ namespace swift {
 
   /// Kind of implicit platform conditions.
   enum class PlatformConditionKind {
-    /// The active os target (OSX, iOS, Linux, etc.)
-    OS,
-    /// The active arch target (x86_64, i386, arm, arm64, etc.)
-    Arch,
-    /// The active endianness target (big or little)
-    Endianness,
-    /// Runtime support (_ObjC or _Native)
-    Runtime,
-    /// Conditional import of module
-    CanImport,
-    /// Target Environment (currently just 'simulator' or absent)
-    TargetEnvironment,
+#define PLATFORM_CONDITION(LABEL, IDENTIFIER) LABEL,
+#include "swift/AST/PlatformConditionKinds.def"
   };
 
   /// Describes which Swift 3 Objective-C inference warnings should be
@@ -437,11 +427,13 @@ namespace swift {
     /// Returns true if the given platform condition argument represents
     /// a supported target operating system.
     ///
-    /// \param suggestions Populated with suggested replacements
+    /// \param suggestedKind Populated with suggested replacement platform condition
+    /// \param suggestedValues Populated with suggested replacement values
     /// if a match is not found.
     static bool checkPlatformConditionSupported(
       PlatformConditionKind Kind, StringRef Value,
-      std::vector<StringRef> &suggestions);
+      PlatformConditionKind &suggestedKind,
+      std::vector<StringRef> &suggestedValues);
 
     /// Return a hash code of any components from these options that should
     /// contribute to a Swift Bridging PCH hash.

lib/Basic/LangOptions.cpp

@@ -65,15 +65,59 @@ static const StringRef SupportedConditionalCompilationTargetEnvironments[] = {
   "simulator",
 };
 
-template <size_t N>
-bool contains(const StringRef (&Array)[N], const StringRef &V,
-              std::vector<StringRef> &suggestions) {
+static const PlatformConditionKind AllPublicPlatformConditionKinds[] = {
+#define PLATFORM_CONDITION(LABEL, IDENTIFIER) PlatformConditionKind::LABEL,
+#define PLATFORM_CONDITION_(LABEL, IDENTIFIER)
+#include "swift/AST/PlatformConditionKinds.def"
+};
+
+ArrayRef<StringRef> getSupportedConditionalCompilationValues(const PlatformConditionKind &Kind) {
+  switch (Kind) {
+  case PlatformConditionKind::OS:
+    return SupportedConditionalCompilationOSs;
+  case PlatformConditionKind::Arch:
+    return SupportedConditionalCompilationArches;
+  case PlatformConditionKind::Endianness:
+    return SupportedConditionalCompilationEndianness;
+  case PlatformConditionKind::Runtime:
+    return SupportedConditionalCompilationRuntimes;
+  case PlatformConditionKind::CanImport:
+    return { };
+  case PlatformConditionKind::TargetEnvironment:
+    return SupportedConditionalCompilationTargetEnvironments;
+  }
+}
+
+PlatformConditionKind suggestedPlatformConditionKind(PlatformConditionKind Kind, const StringRef &V,
+                                                     std::vector<StringRef> &suggestedValues) {
+  std::string lower = V.lower();
+  for (const PlatformConditionKind& candidateKind : AllPublicPlatformConditionKinds) {
+    if (candidateKind != Kind) {
+      auto supportedValues = getSupportedConditionalCompilationValues(candidateKind);
+      for (const StringRef& candidateValue : supportedValues) {
+        if (candidateValue.lower() == lower) {
+          suggestedValues.clear();
+          if (candidateValue != V) {
+            suggestedValues.emplace_back(candidateValue);
+          }
+          return candidateKind;
+        }
+      }
+    }
+  }
+  return Kind;
+}
+
+bool isMatching(PlatformConditionKind Kind, const StringRef &V,
+                PlatformConditionKind &suggestedKind, std::vector<StringRef> &suggestions) {
   // Compare against known values, ignoring case to avoid penalizing
   // characters with incorrect case.
   unsigned minDistance = std::numeric_limits<unsigned>::max();
   std::string lower = V.lower();
-  for (const StringRef& candidate : Array) {
+  auto supportedValues = getSupportedConditionalCompilationValues(Kind);
+  for (const StringRef& candidate : supportedValues) {
     if (candidate == V) {
+      suggestedKind = Kind;
       suggestions.clear();
       return true;
     }
@@ -85,28 +129,21 @@ bool contains(const StringRef (&Array)[N], const StringRef &V,
     if (distance == minDistance)
       suggestions.emplace_back(candidate);
   }
+  suggestedKind = suggestedPlatformConditionKind(Kind, V, suggestions);
   return false;
 }
 
 bool LangOptions::
 checkPlatformConditionSupported(PlatformConditionKind Kind, StringRef Value,
-                                std::vector<StringRef> &suggestions) {
+                                PlatformConditionKind &suggestedKind,
+                                std::vector<StringRef> &suggestedValues) {
   switch (Kind) {
   case PlatformConditionKind::OS:
-    return contains(SupportedConditionalCompilationOSs, Value,
-                    suggestions);
   case PlatformConditionKind::Arch:
-    return contains(SupportedConditionalCompilationArches, Value,
-                    suggestions);
   case PlatformConditionKind::Endianness:
-    return contains(SupportedConditionalCompilationEndianness, Value,
-                    suggestions);
   case PlatformConditionKind::Runtime:
-    return contains(SupportedConditionalCompilationRuntimes, Value,
-                    suggestions);
   case PlatformConditionKind::TargetEnvironment:
-    return contains(SupportedConditionalCompilationTargetEnvironments, Value,
-                    suggestions);
+    return isMatching(Kind, Value, suggestedKind, suggestedValues);
   case PlatformConditionKind::CanImport:
     // All importable names are valid.
     // FIXME: Perform some kind of validation of the string?

lib/Parse/ParseIfConfig.cpp

@@ -38,15 +38,21 @@ namespace {
 static
 Optional<PlatformConditionKind> getPlatformConditionKind(StringRef Name) {
   return llvm::StringSwitch<Optional<PlatformConditionKind>>(Name)
-    .Case("os", PlatformConditionKind::OS)
-    .Case("arch", PlatformConditionKind::Arch)
-    .Case("_endian", PlatformConditionKind::Endianness)
-    .Case("_runtime", PlatformConditionKind::Runtime)
-    .Case("canImport", PlatformConditionKind::CanImport)
-    .Case("targetEnvironment", PlatformConditionKind::TargetEnvironment)
+#define PLATFORM_CONDITION(LABEL, IDENTIFIER) \
+    .Case(IDENTIFIER, PlatformConditionKind::LABEL)
+#include "swift/AST/PlatformConditionKinds.def"
     .Default(None);
 }
 
+/// Get platform condition name from PlatformConditionKind.
+static StringRef getPlatformConditionName(PlatformConditionKind Kind) {
+  switch (Kind) {
+#define PLATFORM_CONDITION(LABEL, IDENTIFIER) \
+  case PlatformConditionKind::LABEL: return IDENTIFIER;
+#include "swift/AST/PlatformConditionKinds.def"
+  }
+}
+
 /// Extract source text of the expression.
 static StringRef extractExprSource(SourceManager &SM, Expr *E) {
   CharSourceRange Range =
@@ -265,9 +271,10 @@ class ValidateIfConfigCondition :
       return nullptr;
     }
 
-    std::vector<StringRef> suggestions;
+    PlatformConditionKind suggestedKind = *Kind;
+    std::vector<StringRef> suggestedValues;
     if (!LangOptions::checkPlatformConditionSupported(*Kind, *ArgStr,
-                                                      suggestions)) {
+                                                      suggestedKind, suggestedValues)) {
       if (Kind == PlatformConditionKind::Runtime) {
         // Error for _runtime()
         D.diagnose(Arg->getLoc(),
@@ -294,7 +301,12 @@ class ValidateIfConfigCondition :
       auto Loc = Arg->getLoc();
       D.diagnose(Loc, diag::unknown_platform_condition_argument,
                  DiagName, *KindName);
-      for (auto suggestion : suggestions)
+      if (suggestedKind != *Kind) {
+        auto suggestedKindName = getPlatformConditionName(suggestedKind);
+        D.diagnose(Loc, diag::note_typo_candidate, suggestedKindName)
+          .fixItReplace(E->getFn()->getSourceRange(), suggestedKindName);
+      }
+      for (auto suggestion : suggestedValues)
         D.diagnose(Loc, diag::note_typo_candidate, suggestion)
           .fixItReplace(Arg->getSourceRange(), suggestion);
     }

test/Parse/ConditionalCompilation/basicParseErrors.swift

@@ -139,6 +139,24 @@ func undefinedFunc() // ignored.
 #endif
 undefinedFunc() // expected-error {{use of unresolved identifier 'undefinedFunc'}}
 
+#if os(simulator) // expected-warning {{unknown operating system for build configuration 'os'}} expected-note {{did you mean 'targetEnvironment'}} {{5-7=targetEnvironment}}
+#endif
+
+#if arch(iOS) // expected-warning {{unknown architecture for build configuration 'arch'}} expected-note {{did you mean 'os'}} {{5-9=os}}
+#endif
+
+#if _endian(arm64) // expected-warning {{unknown endianness for build configuration '_endian'}} expected-note {{did you mean 'arch'}} {{5-12=arch}}
+#endif
+
+#if targetEnvironment(_ObjC) // expected-warning {{unknown target environment for build configuration 'targetEnvironment'}} expected-note {{did you mean 'simulator'}} {{23-28=simulator}}
+#endif
+
+#if os(iOS) || os(simulator) // expected-warning {{unknown operating system for build configuration 'os'}} expected-note {{did you mean 'targetEnvironment'}} {{16-18=targetEnvironment}}
+#endif
+
+#if arch(ios) // expected-warning {{unknown architecture for build configuration 'arch'}} expected-note {{did you mean 'os'}} {{5-9=os}} expected-note {{did you mean 'iOS'}} {{10-13=iOS}}
+#endif
+
 #if FOO
 #else if BAR
 // expected-error@-1 {{unexpected 'if' keyword following '#else' conditional compilation directive; did you mean '#elseif'?}} {{1-9=#elseif}}