Generate unused variable warnings in top level statements

lib/Sema/MiscDiagnostics.cpp

@@ -1854,8 +1854,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
 
 namespace {
 class VarDeclUsageChecker : public ASTWalker {
-  TypeChecker &TC;
-
+  DiagnosticEngine &Diags;
   // Keep track of some information about a variable.
   enum {
     RK_Read        = 1,      ///< Whether it was ever read.
@@ -1883,16 +1882,18 @@ class VarDeclUsageChecker : public ASTWalker {
   void operator=(const VarDeclUsageChecker &) = delete;
 
 public:
-  VarDeclUsageChecker(TypeChecker &TC, AbstractFunctionDecl *AFD) : TC(TC) {
+  VarDeclUsageChecker(TypeChecker &TC, AbstractFunctionDecl *AFD) : Diags(TC.Diags) {
     // Track the parameters of the function.
     for (auto PL : AFD->getParameterLists())
       for (auto param : *PL)
         if (shouldTrackVarDecl(param))
           VarDecls[param] = 0;
 
   }
-
-  VarDeclUsageChecker(TypeChecker &TC, VarDecl *VD) : TC(TC) {
+
+  VarDeclUsageChecker(DiagnosticEngine &Diags) : Diags(Diags) {}
+
+  VarDeclUsageChecker(TypeChecker &TC, VarDecl *VD) : Diags(TC.Diags) {
     // Track a specific VarDecl
     VarDecls[VD] = 0;
   }
@@ -1998,6 +1999,35 @@ class VarDeclUsageChecker : public ASTWalker {
       // Don't walk into it though, it may not even be type checked yet.
       return false;
     }
+    if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(D)) {
+      // If this is a TopLevelCodeDecl, scan for global variables
+      auto *body = TLCD->getBody();
+      for (auto node : body->getElements()) {
+        if (node.is<Decl *>()) {
+          // Flag all variables in a PatternBindingDecl
+          Decl *D = node.get<Decl *>();
+          PatternBindingDecl *PBD = dyn_cast<PatternBindingDecl>(D);
+          if (!PBD) continue;
+          for (PatternBindingEntry PBE : PBD->getPatternList()) {
+            PBE.getPattern()->forEachVariable([&](VarDecl *VD) {
+              VarDecls[VD] = RK_Read|RK_Written;
+            });
+          }
+        } else if (node.is<Stmt *>()) {
+          // Flag all variables in guard statements
+          Stmt *S = node.get<Stmt *>();
+          GuardStmt *GS = dyn_cast<GuardStmt>(S);
+          if (!GS) continue;
+          for (StmtConditionElement SCE : GS->getCond()) {
+            if (auto pattern = SCE.getPatternOrNull()) {
+              pattern->forEachVariable([&](VarDecl *VD) {
+                VarDecls[VD] = RK_Read|RK_Written;
+              });
+            }
+          }
+        }
+      }
+    }
 
 
     // Note that we ignore the initialization behavior of PatternBindingDecls,
@@ -2080,8 +2110,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
       // produce a fixit hint with a parent map, but this is a lot of effort for
       // a narrow case.
       if (access & RK_CaptureList) {
-        TC.diagnose(var->getLoc(), diag::capture_never_used,
-                    var->getName());
+        Diags.diagnose(var->getLoc(), diag::capture_never_used,
+                       var->getName());
         continue;
       }
 
@@ -2097,8 +2127,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
           SourceRange replaceRange(
               pbd->getStartLoc(),
               pbd->getPatternList()[0].getPattern()->getEndLoc());
-          TC.diagnose(var->getLoc(), diag::pbd_never_used,
-                      var->getName(), varKind)
+          Diags.diagnose(var->getLoc(), diag::pbd_never_used,
+                         var->getName(), varKind)
             .fixItReplace(replaceRange, "_");
           continue;
         }
@@ -2132,8 +2162,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
                     noParens = isIsTest = true;
                   }
 
-                  auto diagIF = TC.diagnose(var->getLoc(),
-                                            diag::pbd_never_used_stmtcond,
+                  auto diagIF = Diags.diagnose(var->getLoc(),
+                                               diag::pbd_never_used_stmtcond,
                                             var->getName());
                   auto introducerLoc = SC->getCond()[0].getIntroducerLoc();
                   diagIF.fixItReplaceChars(introducerLoc,
@@ -2160,8 +2190,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
       //    let (a,b) = foo()
       // Just rewrite the one variable with a _.
       unsigned varKind = var->isLet();
-      TC.diagnose(var->getLoc(), diag::variable_never_used,
-                  var->getName(), varKind)
+      Diags.diagnose(var->getLoc(), diag::variable_never_used,
+                     var->getName(), varKind)
         .fixItReplace(var->getLoc(), "_");
       continue;
     }
@@ -2194,18 +2224,18 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
       // If this is a parameter explicitly marked 'var', remove it.
       unsigned varKind = isa<ParamDecl>(var);
       if (FixItLoc.isInvalid())
-        TC.diagnose(var->getLoc(), diag::variable_never_mutated,
-                    var->getName(), varKind);
+        Diags.diagnose(var->getLoc(), diag::variable_never_mutated,
+                       var->getName(), varKind);
       else
-        TC.diagnose(var->getLoc(), diag::variable_never_mutated,
-                    var->getName(), varKind)
+        Diags.diagnose(var->getLoc(), diag::variable_never_mutated,
+                       var->getName(), varKind)
           .fixItReplace(FixItLoc, "let");
       continue;
     }
 
     // If this is a variable that was only written to, emit a warning.
     if ((access & RK_Read) == 0) {
-      TC.diagnose(var->getLoc(), diag::variable_never_read, var->getName(),
+      Diags.diagnose(var->getLoc(), diag::variable_never_read, var->getName(),
                   isa<ParamDecl>(var));
       continue;
     }
@@ -2376,6 +2406,14 @@ void VarDeclUsageChecker::handleIfConfig(IfConfigStmt *ICS) {
   }
 }
 
+/// Apply the warnings managed by VarDeclUsageChecker to the top level
+/// code declarations that haven't been checked yet.
+void swift::
+performTopLevelDeclDiagnostics(TypeChecker &TC, TopLevelCodeDecl *TLCD) {
+  VarDeclUsageChecker checker(TC.Diags);
+  TLCD->walk(checker);
+}
+
 /// Perform diagnostics for func/init/deinit declarations.
 void swift::performAbstractFuncDeclDiagnostics(TypeChecker &TC,
                                                AbstractFunctionDecl *AFD) {

lib/Sema/MiscDiagnostics.h

@@ -28,6 +28,7 @@ namespace swift {
   class Expr;
   class InFlightDiagnostic;
   class Stmt;
+  class TopLevelCodeDecl;
   class TypeChecker;
   class ValueDecl;
 
@@ -41,6 +42,9 @@ void performStmtDiagnostics(TypeChecker &TC, const Stmt *S);
 
 void performAbstractFuncDeclDiagnostics(TypeChecker &TC,
                                         AbstractFunctionDecl *AFD);
+
+/// Perform diagnostics on the top level code declaration.
+void performTopLevelDeclDiagnostics(TypeChecker &TC, TopLevelCodeDecl *TLCD);
 
 /// Emit a fix-it to set the accessibility of \p VD to \p desiredAccess.
 ///

lib/Sema/TypeCheckStmt.cpp

@@ -1585,4 +1585,5 @@ void TypeChecker::typeCheckTopLevelCodeDecl(TopLevelCodeDecl *TLCD) {
   StmtChecker(*this, TLCD).typeCheckStmt(Body);
   TLCD->setBody(Body);
   checkTopLevelErrorHandling(TLCD);
+  performTopLevelDeclDiagnostics(*this, TLCD);
 }

lib/Sema/TypeChecker.cpp

@@ -17,6 +17,7 @@
 
 #include "swift/Subsystems.h"
 #include "TypeChecker.h"
+#include "MiscDiagnostics.h"
 #include "GenericTypeResolver.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/ASTVisitor.h"

test/ClangImporter/blocks_parse.swift

@@ -32,7 +32,7 @@ func checkTypeImpl<T>(_ a: inout T, _: T.Type) {}
 do {
   var blockOpt = blockWithoutNullability()
   checkTypeImpl(&blockOpt, Optional<my_block_t>.self)
-  var block: my_block_t = blockWithoutNullability()
+  var _: my_block_t = blockWithoutNullability()
 }
 do {
   var block = blockWithNonnull()
@@ -41,7 +41,7 @@ do {
 do {
   var blockOpt = blockWithNullUnspecified()
   checkTypeImpl(&blockOpt, Optional<my_block_t>.self)
-  var block: my_block_t = blockWithNullUnspecified()
+  var _: my_block_t = blockWithNullUnspecified()
 }
 do {
   var block = blockWithNullable()

test/Compatibility/tuple_arguments.swift

@@ -53,10 +53,10 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var c = (3)
-  var d = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var c = (3) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
+  var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
 
   concrete(a)
   concrete((a))
@@ -535,9 +535,9 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var c = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
 
   _ = InitTwo(a, b)
   _ = InitTwo((a, b)) // expected-error {{missing argument for parameter #2 in call}}
@@ -583,9 +583,9 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var d = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
 
   var s1 = SubscriptTwo()
   _ = s1[a, b]
@@ -952,9 +952,9 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var c = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
 
   // _ = GenericInit<(Int, Int)>(a, b) // Crashes in Swift 3
   _ = GenericInit<(Int, Int)>((a, b)) // expected-error {{expression type 'GenericInit<(Int, Int)>' is ambiguous without more context}}
@@ -1017,9 +1017,9 @@ do {
 }
 
 do {
-  var a = 3.0
-  var b = 4.0
-  var d = (a, b)
+  var a = 3.0 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4.0 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
 
   var s1 = GenericSubscript<(Double, Double)>()
   _ = s1[a, b]

test/Constraints/operator.swift

@@ -6,7 +6,7 @@ do {
   let a: String? = "a"
   let b: String? = "b"
   let c: String? = "c"
-  let d: String? = a! + b! + c!
+  let _: String? = a! + b! + c!
 
   let x: Double = 1
   _ = x + x + x

test/Constraints/patterns.swift

@@ -123,7 +123,7 @@ default:
 
 // <rdar://problem/19382878> Introduce new x? pattern
 switch Optional(42) {
-case let x?: break
+case let x?: break // expected-warning{{immutable value 'x' was never used; consider replacing with '_' or removing it}}
 case nil: break
 }
 
@@ -181,9 +181,9 @@ case x ?? 42: break // match value
 default: break
 }
 
-for (var x) in 0...100 {}
-for var x in 0...100 {}  // rdar://20167543
-for (let x) in 0...100 {} // expected-error {{'let' pattern cannot appear nested in an already immutable context}}
+for (var x) in 0...100 {} // expected-warning{{variable 'x' was never used; consider replacing with '_' or removing it}}
+for var x in 0...100 {}  // rdar://20167543 expected-warning{{variable 'x' was never used; consider replacing with '_' or removing it}}
+for (let x) in 0...100 { _ = x} // expected-error {{'let' pattern cannot appear nested in an already immutable context}}
 
 var (let y) = 42  // expected-error {{'let' cannot appear nested inside another 'var' or 'let' pattern}}
 let (var z) = 42  // expected-error {{'var' cannot appear nested inside another 'var' or 'let' pattern}}

test/Constraints/tuple_arguments.swift

@@ -37,10 +37,10 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var c = (3)
-  var d = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var c = (3) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
+  var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
 
   concrete(a)
   concrete((a))
@@ -520,9 +520,9 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var c = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
 
   _ = InitTwo(a, b)
   _ = InitTwo((a, b)) // expected-error {{missing argument for parameter #2 in call}}
@@ -568,9 +568,9 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var d = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
 
   var s1 = SubscriptTwo()
   _ = s1[a, b]
@@ -937,9 +937,9 @@ do {
 }
 
 do {
-  var a = 3
-  var b = 4
-  var c = (a, b)
+  var a = 3 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var c = (a, b) // expected-warning {{variable 'c' was never mutated; consider changing to 'let' constant}}
 
   _ = GenericInit<(Int, Int)>(a, b) // expected-error {{extra argument in call}}
   _ = GenericInit<(Int, Int)>((a, b))
@@ -1002,9 +1002,9 @@ do {
 }
 
 do {
-  var a = 3.0
-  var b = 4.0
-  var d = (a, b)
+  var a = 3.0 // expected-warning {{variable 'a' was never mutated; consider changing to 'let' constant}}
+  var b = 4.0 // expected-warning {{variable 'b' was never mutated; consider changing to 'let' constant}}
+  var d = (a, b) // expected-warning {{variable 'd' was never mutated; consider changing to 'let' constant}}
 
   var s1 = GenericSubscript<(Double, Double)>()
   _ = s1[a, b] // expected-error {{extra argument in call}}

test/Parse/availability_query.swift

@@ -97,7 +97,7 @@ if 1 != 2, #available(iOS 8.0, *) {}
 
 // Pattern then #available(iOS 8.0, *) {
 if case 42 = 42, #available(iOS 8.0, *) {}
-if let x = Optional(42), #available(iOS 8.0, *) {}
+if let _ = Optional(42), #available(iOS 8.0, *) {}
 
 // Allow "macOS" as well.
 if #available(macOS 10.51, *) {

test/Parse/brace_recovery_eof.swift

@@ -2,5 +2,5 @@
 
 // Make sure source ranges satisfy the verifier.
 for foo in [1, 2] { // expected-note {{to match this opening '{'}}
-  var x = foo
+  _ = foo
 // expected-error@+1{{expected '}' at end of brace statement}}