Merge branch 'main' into release/2.x

nix-community · Aug 7, 2024 · 1f94b8e · 1f94b8e
2 parents 4e9d406 + af1255e
commit 1f94b8e
Show file tree

Hide file tree

Showing 16 changed files with 345 additions and 93 deletions.
diff --git a/.github/workflows/treefmt.yml b/.github/workflows/treefmt.yml
@@ -0,0 +1,47 @@
+name: Check formatting with treefmt
+
+on:
+  [push, pull_request]
+
+jobs:
+  treefmt:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: cachix/install-nix-action@v22
+    - uses: actions/checkout@v4
+    - id: changed-files
+      uses: tj-actions/changed-files@v44
+    - name: Run treefmt
+      run: nix fmt
+    - name: Check diff
+      env:
+        ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
+      run: |
+        unformatted_touched=()
+        unformatted_untouched=()
+        while IFS= read -r unformatted_file; do
+          matched=
+          for changed_file in ${ALL_CHANGED_FILES}; do
+            if [[ "$changed_file" == "$unformatted_file" ]]; then
+              unformatted_touched+=("$unformatted_file")
+              matched=1
+              break
+            fi
+          done
+          if [[ -z "$matched" ]]; then
+            unformatted_untouched+=("$unformatted_file")
+          fi
+        done <<< "$(git diff --name-only)"
+
+        if (( ${#unformatted_untouched[@]} )); then
+          echo "These files are not formatted, but out of scope of this PR:"
+          printf '%s\n' "${unformatted_untouched[@]}"
+        fi
+
+        echo # blank line
+
+        if (( ${#unformatted_touched[@]} )); then
+          echo "These files are created/edited but not formatted:"
+          printf '%s\n' "${unformatted_touched[@]}"
+          exit 1
+        fi
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,6 @@
 /html/
 
 __pycache__
+
+result
+result-*
diff --git a/README.md b/README.md
@@ -29,4 +29,3 @@ We have tested some working & reproducible [editor environments](/nixd/docs/edit
 - [Configuration](nixd/docs/configuration.md) (see how to, and which options are tunable)
 - [Features](nixd/docs/features.md) (features explanation)
 - [Developers' Manual](nixd/docs/dev.md) (internal design, contributing)
-- Project matrix room: https://matrix.to/#/#nixd:matrix.org
diff --git a/flake.lock b/flake.lock
diff --git a/flake.nix b/flake.nix
@@ -5,9 +5,14 @@
     flake-parts.url = "github:hercules-ci/flake-parts";
 
     flake-root.url = "github:srid/flake-root";
+
+    treefmt-nix = {
+      url = "github:numtide/treefmt-nix";
+      inputs.nixpkgs.follows = "nixpkgs";
+    };
   };
 
-  outputs = { nixpkgs, flake-parts, ... }@inputs: flake-parts.lib.mkFlake { inherit inputs; } {
+  outputs = { nixpkgs, flake-parts, treefmt-nix, ... }@inputs: flake-parts.lib.mkFlake { inherit inputs; } {
     imports = [
       inputs.flake-parts.flakeModules.easyOverlay
       inputs.flake-root.flakeModule
@@ -45,6 +50,7 @@
           '';
           hardeningDisable = [ "fortify" ];
         };
+        treefmtEval = treefmt-nix.lib.evalModule pkgs ./treefmt.nix;
       in
       {
         packages.default = nixd;
@@ -103,6 +109,7 @@
             (import ./nixd/docs/editors/vscodium.nix { inherit pkgs; })
           ];
         };
+        formatter = treefmtEval.config.build.wrapper;
       };
     systems = nixpkgs.lib.systems.flakeExposed;
   };

diff --git a/libnixf/include/nixf/Sema/VariableLookup.h b/libnixf/include/nixf/Sema/VariableLookup.h
@@ -36,8 +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 (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,21 +184,39 @@ 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",
         "cname": "ExtraRecursive",
-        "severity": "Hint",
-        "message": "attrset is not necessary to be `rec`ursive ",
+        "severity": "Warning",
+        "message": "attrset is not necessary to be `rec`ursive",
     },
     {
         "sname": "sema-extra-with",
         "cname": "ExtraWith",
-        "severity": "Hint",
+        "severity": "Warning",
         "message": "unused `with` expression",
     },
     {

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);
     }
@@ -129,20 +143,38 @@ void VariableLookupAnalysis::dfs(const ExprLambda &Lambda,
   // foo: body
   // ^~~<------- add function argument.
   if (Arg.id()) {
-    // Function arg cannot duplicate to it's formal.
-    // If it this unluckily happens, we would like to skip this definition.
-    if (!Arg.formals() || !Arg.formals()->dedup().contains(Arg.id()->name()))
+    if (!Arg.formals()) {
       ToDef.insert_or_assign(Arg.id(), DBuilder.add(Arg.id()->name(), Arg.id(),
                                                     Definition::DS_LambdaArg));
+      // Function arg cannot duplicate to it's formal.
+      // If it this unluckily happens, we would like to skip this definition.
+    } else if (!Arg.formals()->dedup().contains(Arg.id()->name())) {
+      ToDef.insert_or_assign(Arg.id(),
+                             DBuilder.add(Arg.id()->name(), Arg.id(),
+                                          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) {
@@ -172,6 +170,31 @@ TEST_F(VLATest, LivenessDupSymbol) {
   ASSERT_EQ(Diags[0].tags().size(), 0);
 }
 
+TEST_F(VLATest, LivenessArgWithFormal) {
+  std::shared_ptr<Node> AST = parse("{ foo }@bar: foo", Diags);
+  VariableLookupAnalysis VLA(Diags);
+  VLA.runOnAST(*AST);
+
+  ASSERT_EQ(Diags.size(), 1);
+
+  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);
+}
+
 TEST_F(VLATest, ToDefAttrs) {
   std::shared_ptr<Node> AST = parse("rec { x = 1; y = x; z = x; }", Diags);
   VariableLookupAnalysis VLA(Diags);
@@ -271,10 +294,11 @@ TEST_F(VLATest, EscapingWith) {
 }
 
 TEST_F(VLATest, EscapingWithButBuiltin) {
-  std::shared_ptr<Node> AST = parse("with { a = 1; }; [ a true false null ]", Diags);
+  std::shared_ptr<Node> AST =
+      parse("with { a = 1; }; [ a true false null ]", Diags);
   VariableLookupAnalysis VLA(Diags);
   VLA.runOnAST(*AST);
-  
+
   ASSERT_EQ(Diags.size(), 0);
 }