libnixf: split sema-unused-def into let, arg and formal (#565)

Related NixOS/nixpkgs#331085 Fixes: #558 --------- Co-authored-by: Yingchi Long <i@lyc.dev>
nix-community · Aug 2, 2024 · d687efa · d687efa
1 parent 187aabe
commit d687efa
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 26 deletions.
diff --git a/libnixf/include/nixf/Sema/VariableLookup.h b/libnixf/include/nixf/Sema/VariableLookup.h
@@ -36,11 +36,16 @@ class Definition {
     /// \brief From ambda arg e.g.  a: a + 1
     DS_LambdaArg,
 
-    /// \brief From lambda formal, e.g. { a }: a + 1
-    DS_LambdaFormal,
+    /// \brief From lambda (noarg) formal, e.g. { a }: a + 1
+    DS_LambdaNoArg_Formal,
 
-    /// \brief From lambda arg with formal, e.g. { foo, bar }@a: a + 1
-    DS_LambdaArgWithFormal,
+    /// \brief From lambda (with `@arg`) `arg`,
+    /// e.g. `a` in `{ foo }@a: foo + 1`
+    DS_LambdaWithArg_Arg,
+
+    /// \brief From lambda (with `@arg`) formal,
+    /// e.g. `foo` in `{ foo }@a: foo + 1`
+    DS_LambdaWithArg_Formal,
 
     /// \brief From recursive attribute set.  e.g. rec { }
     DS_Rec,

diff --git a/libnixf/src/Basic/diagnostic.py b/libnixf/src/Basic/diagnostic.py
@@ -184,10 +184,28 @@ class Diagnostic(TypedDict):
         "message": "undefined variable `{}`",
     },
     {
-        "sname": "sema-def-not-used",
-        "cname": "DefinitionNotUsed",
+        "sname": "sema-unused-def-let",
+        "cname": "UnusedDefLet",
+        "severity": "Warning",
+        "message": "definition `{}` in let-expression is not used",
+    },
+    {
+        "sname": "sema-unused-def-lambda-noarg-formal",
+        "cname": "UnusedDefLambdaNoArg_Formal",
+        "severity": "Warning",
+        "message": "attribute `{}` of argument is not used",
+    },
+    {
+        "sname": "sema-unused-def-lambda-witharg-formal",
+        "cname": "UnusedDefLambdaWithArg_Formal",
+        "severity": "Warning",
+        "message": "argument `{}` in `@`-pattern is not used",
+    },
+    {
+        "sname": "sema-unused-def-lambda-witharg-arg",
+        "cname": "UnusedDefLambdaWithArg_Arg",
         "severity": "Hint",
-        "message": "definition `{}` is not used",
+        "message": "attribute `{}` of `@`-pattern argument is not used, but may be referenced from the argument",
     },
     {
         "sname": "sema-extra-rec",

diff --git a/libnixf/src/Sema/VariableLookup.cpp b/libnixf/src/Sema/VariableLookup.cpp
@@ -52,8 +52,22 @@ void VariableLookupAnalysis::emitEnvLivenessWarning(
     if (!Def->syntax())
       continue;
     if (Def->uses().empty()) {
-      Diagnostic &D = Diags.emplace_back(Diagnostic::DK_DefinitionNotUsed,
-                                         Def->syntax()->range());
+      Diagnostic::DiagnosticKind Kind = [&]() {
+        switch (Def->source()) {
+        case Definition::DS_Let:
+          return Diagnostic::DK_UnusedDefLet;
+        case Definition::DS_LambdaNoArg_Formal:
+          return Diagnostic::DK_UnusedDefLambdaNoArg_Formal;
+        case Definition::DS_LambdaWithArg_Formal:
+          return Diagnostic::DK_UnusedDefLambdaWithArg_Formal;
+        case Definition::DS_LambdaWithArg_Arg:
+          return Diagnostic::DK_UnusedDefLambdaWithArg_Arg;
+        default:
+          assert(false && "liveness diagnostic encountered an unknown source!");
+          __builtin_unreachable();
+        }
+      }();
+      Diagnostic &D = Diags.emplace_back(Kind, Def->syntax()->range());
       D << Name;
       D.tag(DiagnosticTag::Faded);
     }
@@ -137,17 +151,30 @@ void VariableLookupAnalysis::dfs(const ExprLambda &Lambda,
     } else if (!Arg.formals()->dedup().contains(Arg.id()->name())) {
       ToDef.insert_or_assign(Arg.id(),
                              DBuilder.add(Arg.id()->name(), Arg.id(),
-                                          Definition::DS_LambdaArgWithFormal));
+                                          Definition::DS_LambdaWithArg_Arg));
     }
   }
 
   // { foo, bar, ... } : body
-  ///  ^~~~~~~~~<--------------  add function formals.
-  if (Arg.formals())
-    for (const auto &[Name, Formal] : Arg.formals()->dedup())
-      ToDef.insert_or_assign(
-          Formal->id(),
-          DBuilder.add(Name, Formal->id(), Definition::DS_LambdaFormal));
+  //   ^~~~~~~~~<--------------  add function formals.
+
+  // This section differentiates between formal parameters with an argument and
+  // without. Example:
+  //
+  //  { foo }@arg : use arg
+  //
+  // In this case, the definition of `foo` is not used directly; however, it
+  // might be accessed via arg.foo. Therefore, the severity of an unused formal
+  // parameter is reduced in this scenario.
+  if (Arg.formals()) {
+    for (const auto &[Name, Formal] : Arg.formals()->dedup()) {
+      Definition::DefinitionSource Source =
+          Arg.id() ? Definition::DS_LambdaWithArg_Formal
+                   : Definition::DS_LambdaNoArg_Formal;
+      ToDef.insert_or_assign(Formal->id(),
+                             DBuilder.add(Name, Formal->id(), Source));
+    }
+  }
 
   auto NewEnv = std::make_shared<EnvNode>(Env, DBuilder.finish(), &Lambda);
 

diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp
@@ -135,29 +135,27 @@ TEST_F(VLATest, LivenessRec) {
   ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_ExtraRecursive);
 }
 
-TEST_F(VLATest, LivenessArg) {
+TEST_F(VLATest, LivenessFormal) {
   std::shared_ptr<Node> AST = parse("{ foo }: 1", Diags);
   VariableLookupAnalysis VLA(Diags);
   VLA.runOnAST(*AST);
 
   ASSERT_EQ(Diags.size(), 1);
 
-  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed);
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaNoArg_Formal);
   ASSERT_EQ(Diags[0].tags().size(), 1);
-  ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded);
 }
 
-TEST_F(VLATest, LivenessNested) {
+TEST_F(VLATest, LivenessLet) {
   std::shared_ptr<Node> AST = parse("let y = 1; in x: y: x + y", Diags);
   VariableLookupAnalysis VLA(Diags);
   VLA.runOnAST(*AST);
 
   ASSERT_EQ(Diags.size(), 1);
 
-  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed);
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLet);
   ASSERT_EQ(Diags[0].range().lCur().column(), 4);
   ASSERT_EQ(Diags[0].tags().size(), 1);
-  ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded);
 }
 
 TEST_F(VLATest, LivenessDupSymbol) {
@@ -179,9 +177,21 @@ TEST_F(VLATest, LivenessArgWithFormal) {
 
   ASSERT_EQ(Diags.size(), 1);
 
-  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed);
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaWithArg_Arg);
   ASSERT_EQ(Diags[0].range().lCur().column(), 8);
   ASSERT_EQ(Diags[0].tags().size(), 1);
+}
+
+TEST_F(VLATest, LivenessFormalWithArg) {
+  std::shared_ptr<Node> AST = parse("{ foo }@bar: bar", Diags);
+  VariableLookupAnalysis VLA(Diags);
+  VLA.runOnAST(*AST);
+
+  ASSERT_EQ(Diags.size(), 1);
+
+  ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaWithArg_Formal);
+  ASSERT_EQ(Diags[0].range().lCur().column(), 2);
+  ASSERT_EQ(Diags[0].tags().size(), 1);
   ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded);
 }
 

diff --git a/nixd/tools/nixd/test/diagnostic/liveness-formal.md b/nixd/tools/nixd/test/diagnostic/liveness-formal.md
@@ -38,8 +38,8 @@
 ```
      CHECK: "diagnostics": [
 CHECK-NEXT:   {
-CHECK-NEXT:     "code": "sema-def-not-used",
-CHECK-NEXT:     "message": "definition `y` is not used",
+CHECK-NEXT:     "code": "sema-unused-def-lambda-noarg-formal",
+CHECK-NEXT:     "message": "attribute `y` of argument is not used",
 CHECK-NEXT:     "range": {
 CHECK-NEXT:       "end": {
 CHECK-NEXT:         "character": 5,
@@ -51,7 +51,7 @@ CHECK-NEXT:         "line": 0
 CHECK-NEXT:       }
 CHECK-NEXT:     },
 CHECK-NEXT:     "relatedInformation": [],
-CHECK-NEXT:     "severity": 4,
+CHECK-NEXT:     "severity": 2,
 CHECK-NEXT:     "source": "nixf",
 CHECK-NEXT:     "tags": [
 CHECK-NEXT:       1