Merge pull request swiftlang#30302 from CodaFi/all-the-fixins

Teach the Dependency Verifier to Emit Fixits

Author: CodaFi

include/swift/AST/DiagnosticsFrontend.def

@@ -396,7 +396,7 @@ ERROR(missing_member_dependency,none,
       "dependency does not exist: %1",
       (/*Expectation::Kind*/uint8_t, StringRef))
 ERROR(unexpected_dependency,none,
-      "unexpected %0 %select{%error|%error||potential member|dynamic member}1 "
+      "unexpected %0 %select{%error|%error|member|potential member|dynamic member}1 "
       "dependency: %2", (StringRef, /*Expectation::Kind*/uint8_t, StringRef))
 ERROR(unexpected_provided_entity,none,
       "unexpected provided entity: %0", (StringRef))

lib/Frontend/DependencyVerifier.cpp

@@ -224,6 +224,7 @@ struct Obligation {
            "Cannot form negative obligation!");
   }
 
+  Expectation::Scope getScope() const { return info.second; }
   Expectation::Kind getKind() const { return info.first; }
   StringRef getName() const { return name; }
   bool getCascades() const {
@@ -240,6 +241,22 @@ struct Obligation {
     }
   }
 
+  StringRef renderAsFixit(ASTContext &Ctx) const {
+    llvm::StringRef selector =
+#define MATRIX_ENTRY(SELECTOR, SCOPE, KIND) \
+    if (getKind() == Expectation::Kind::KIND && \
+        getScope() == Expectation::Scope::SCOPE) { \
+      return SELECTOR; \
+    }
+
+    [this]() -> StringRef {
+      EXPECTATION_MATRIX
+      return "";
+    }();
+#undef MATRIX_ENTRY
+    return Ctx.AllocateCopy(("// " + selector + "{{" + getName() + "}}").str());
+  }
+
 public:
   bool isOwed() const { return state == State::Owed; }
   FullfillmentToken fullfill() {
@@ -439,17 +456,19 @@ bool DependencyVerifier::constructObligations(const SourceFile *SF,
           auto demContext = copyDemangledTypeName(Ctx, context);
           auto key = Ctx.AllocateCopy((demContext + "." + name).str());
           Obligations.insert({Obligation::Key::forMember(key),
-                              {context, Expectation::Kind::Member,
+                              {key, Expectation::Kind::Member,
                                isCascadingUse ? Expectation::Scope::Cascading
                                               : Expectation::Scope::Private}});
         }
           break;
-        case NodeKind::dynamicLookup:
+        case NodeKind::dynamicLookup: {
+          auto contextCpy = Ctx.AllocateCopy(context);
           Obligations.insert({Obligation::Key::forDynamicMember(name),
-                              {context, Expectation::Kind::DynamicMember,
+                              {contextCpy, Expectation::Kind::DynamicMember,
                                isCascadingUse ? Expectation::Scope::Cascading
                                               : Expectation::Scope::Private}});
           break;
+        }
         case NodeKind::topLevel:
         case NodeKind::sourceFileProvide:
           Obligations.insert({Obligation::Key::forProvides(name),
@@ -564,6 +583,7 @@ bool DependencyVerifier::diagnoseUnfulfilledObligations(
   CharSourceRange EntireRange = SM.getRangeForBuffer(*SF->getBufferID());
   StringRef InputFile = SM.extractText(EntireRange);
   auto &diags = SF->getASTContext().Diags;
+  auto &Ctx = SF->getASTContext();
   forEachOwedObligation(Obligations, [&](StringRef key, Obligation &p) {
     // HACK: Diagnosing the end of the buffer will print a carat pointing
     // at the file path, but not print any of the buffer's contents, which
@@ -576,10 +596,12 @@ bool DependencyVerifier::diagnoseUnfulfilledObligations(
     case Expectation::Kind::DynamicMember:
     case Expectation::Kind::PotentialMember:
       diags.diagnose(Loc, diag::unexpected_dependency, p.describeCascade(),
-                     static_cast<uint8_t>(p.getKind()), key);
+                     static_cast<uint8_t>(p.getKind()), key)
+        .fixItInsert(Loc, p.renderAsFixit(Ctx));
       break;
     case Expectation::Kind::Provides:
-      diags.diagnose(Loc, diag::unexpected_provided_entity, p.getName());
+      diags.diagnose(Loc, diag::unexpected_provided_entity, p.getName())
+        .fixItInsert(Loc, p.renderAsFixit(Ctx));
       break;
     }
   });

test/Incremental/fail/main-windows.swift

@@ -0,0 +1,13 @@
+// REQUIRES: OS=windows-msvc
+// This test is special-cased on Windows because dependencies in Two.swift don't
+// appear to be registered against the tracker. rdar://60030114
+
+// RUN: %empty-directory(%t)
+// RUN: %{python} %S/../gen-output-file-map.py -o %t %S/Inputs -r %t.resp
+// RUN: cd %t
+// RUN: not %target-swiftc_driver -no-color-diagnostics -typecheck -output-file-map %t/output.json -incremental -module-name main -verify-incremental-dependencies @%t.resp 2>&1 | sort | %FileCheck %s
+
+// CHECK: unexpected cascading member dependency: main.Base.init
+// CHECK: unexpected cascading potential member dependency: main.Base
+// CHECK: unexpected provided entity: Base
+// CHECK: unexpected provided entity: BaseProtocol

test/Incremental/fail/main.swift

@@ -1,14 +1,15 @@
-// REQUIRES: rdar60030114
+// UNSUPPORTED: OS=windows-msvc
+// See its counterpart main-windows.swift
 
 // RUN: %empty-directory(%t)
 // RUN: %{python} %S/../gen-output-file-map.py -o %t %S/Inputs -r %t.resp
 // RUN: cd %t
 // RUN: not %target-swiftc_driver -no-color-diagnostics -typecheck -output-file-map %t/output.json -incremental -module-name main -verify-incremental-dependencies @%t.resp 2>&1 | sort | %FileCheck %s
 
 // CHECK: unexpected dependency exists: main.BaseProtocol
-// CHECK: unexpected cascading dependency: main.Base.init
-// CHECK: unexpected cascading dependency: main.Subclass.deinit
-// CHECK: unexpected cascading dependency: main.Subclass.init
+// CHECK: unexpected cascading member dependency: main.Base.init
+// CHECK: unexpected cascading member dependency: main.Subclass.deinit
+// CHECK: unexpected cascading member dependency: main.Subclass.init
 // CHECK: unexpected cascading potential member dependency: main.Base
 // CHECK: unexpected provided entity: Base
 // CHECK: unexpected provided entity: BaseProtocol

test/Incremental/gen-output-file-map.py

@@ -61,11 +61,11 @@ def main(arguments):
         'swift-dependencies': './main-buildrecord.swiftdeps'
     }
 
-    with open(output_path, 'w') as f:
+    with open(output_path, 'wb') as f:
         json.dump(all_records, f)
 
     if args.response_output_file is not None:
-        with open(args.response_output_file, 'w') as f:
+        with open(args.response_output_file, 'wb') as f:
             for line in response_file_contents:
                 f.write(line + " ")