From d687efae4a9931ac08e718712cf69de322473847 Mon Sep 17 00:00:00 2001 From: Aleksana Date: Fri, 2 Aug 2024 15:42:40 +0800 Subject: [PATCH] libnixf: split sema-unused-def into let, arg and formal (#565) Related https://github.com/NixOS/nixpkgs/issues/331085 Fixes: #558 --------- Co-authored-by: Yingchi Long --- libnixf/include/nixf/Sema/VariableLookup.h | 13 ++++-- libnixf/src/Basic/diagnostic.py | 24 ++++++++-- libnixf/src/Sema/VariableLookup.cpp | 45 +++++++++++++++---- libnixf/test/Sema/VariableLookup.cpp | 24 +++++++--- .../nixd/test/diagnostic/liveness-formal.md | 6 +-- 5 files changed, 86 insertions(+), 26 deletions(-) diff --git a/libnixf/include/nixf/Sema/VariableLookup.h b/libnixf/include/nixf/Sema/VariableLookup.h index e470415cd..7b0843f3e 100644 --- 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 index 57d4fe837..ecaef03ea 100644 --- 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 index d25d3c3d4..9b3b38ab0 100644 --- 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(Env, DBuilder.finish(), &Lambda); diff --git a/libnixf/test/Sema/VariableLookup.cpp b/libnixf/test/Sema/VariableLookup.cpp index ef11bde0c..b48a78784 100644 --- 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 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 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 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 index f90dd5b71..6391524cb 100644 --- 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