Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions libnixf/include/nixf/Sema/VariableLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ class VariableLookupAnalysis {

std::map<const ExprVar *, LookupResult> Results;

/// \brief For variables resolved from `with` scopes, track all enclosing
/// `with` expressions that could potentially provide the binding.
/// This is needed to detect indirect nested `with` scenarios where
/// converting an outer `with` to `let/inherit` could change semantics.
std::map<const ExprVar *, std::vector<const ExprWith *>> VarWithScopes;

public:
VariableLookupAnalysis(std::vector<Diagnostic> &Diags);

Expand Down Expand Up @@ -198,6 +204,28 @@ class VariableLookupAnalysis {
}

const EnvNode *env(const Node *N) const;

/// \brief Get all `with` expressions that could provide the binding for a
/// variable.
///
/// For variables resolved from `with` scopes, multiple enclosing `with`
/// expressions may potentially provide the binding. This method returns
/// all such `with` expressions, ordered from innermost to outermost.
///
/// This is useful for detecting indirect nested `with` scenarios where
/// converting an outer `with` to `let/inherit` could change semantics.
///
/// \param Var The variable to query.
/// \return A vector of `ExprWith` pointers representing all `with` scopes
/// that could provide the variable's binding. Returns an empty
/// vector if the variable is not resolved from a `with` scope.
[[nodiscard]] std::vector<const ExprWith *>
getWithScopes(const ExprVar &Var) const {
auto It = VarWithScopes.find(&Var);
if (It != VarWithScopes.end())
return It->second;
return {};
}
};

} // namespace nixf
6 changes: 6 additions & 0 deletions libnixf/src/Sema/VariableLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,16 @@ void VariableLookupAnalysis::lookupVar(const ExprVar &Var,
Def->usedBy(Var);
Results.insert({&Var, LookupResult{LookupResultKind::Defined, Def}});
} else if (!WithEnvs.empty()) { // comes from enclosed "with" expressions.
// Collect all `with` expressions that could provide this variable's
// binding. This is stored for later queries (e.g., by code actions that
// need to determine if converting a `with` to `let/inherit` is safe).
std::vector<const ExprWith *> WithScopes;
for (const auto *WithEnv : WithEnvs) {
Def = WithDefs.at(WithEnv->syntax());
Def->usedBy(Var);
WithScopes.push_back(static_cast<const ExprWith *>(WithEnv->syntax()));
}
VarWithScopes.insert({&Var, std::move(WithScopes)});
Results.insert({&Var, LookupResult{LookupResultKind::FromWith, Def}});
} else {
// Check if this is a primop.
Expand Down
146 changes: 146 additions & 0 deletions libnixf/test/Sema/VariableLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,4 +581,150 @@ TEST_F(VLATest, PrimOp_Override_Namespace) {
ASSERT_EQ(Diags.size(), 0);
}

//===----------------------------------------------------------------------===//
// getWithScopes() tests - for detecting nested with scopes
//===----------------------------------------------------------------------===//

TEST_F(VLATest, GetWithScopes_SingleWith) {
// A single `with` scope should return exactly one element
std::shared_ptr<Node> AST = parse("with lib; foo", Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_TRUE(AST);
const auto &With = *static_cast<const ExprWith *>(AST.get());
const auto &Var = *static_cast<const ExprVar *>(With.expr());

auto Scopes = VLA.getWithScopes(Var);
ASSERT_EQ(Scopes.size(), 1);
ASSERT_EQ(Scopes[0], &With);
}

TEST_F(VLATest, GetWithScopes_DirectNested) {
// Direct nested with: variable should have 2 scopes (inner first, outer last)
const char *Src = R"(with outer; with inner; foo)";
std::shared_ptr<Node> AST = parse(Src, Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_TRUE(AST);
const auto &OuterWith = *static_cast<const ExprWith *>(AST.get());
const auto &InnerWith = *static_cast<const ExprWith *>(OuterWith.expr());
const auto &Var = *static_cast<const ExprVar *>(InnerWith.expr());

auto Scopes = VLA.getWithScopes(Var);
ASSERT_EQ(Scopes.size(), 2);
// Innermost first
ASSERT_EQ(Scopes[0], &InnerWith);
ASSERT_EQ(Scopes[1], &OuterWith);
}

TEST_F(VLATest, GetWithScopes_IndirectNested_Let) {
// Indirect nested with through let: `with a; let x = with b; foo; in x`
// The variable `foo` should have 2 scopes
const char *Src = R"(with outer; let x = with inner; foo; in x)";
std::shared_ptr<Node> AST = parse(Src, Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_TRUE(AST);
// Navigate: ExprWith -> ExprLet -> Binds -> Binding[0].Value -> ExprWith
// -> ExprVar
const auto &OuterWith = *static_cast<const ExprWith *>(AST.get());
const auto *Let = static_cast<const ExprLet *>(OuterWith.expr());
ASSERT_TRUE(Let);

// Find the inner with by navigating the AST
// The binding value should be the inner with
const auto *Binds = Let->binds();
ASSERT_TRUE(Binds);
const auto &Bindings = Binds->bindings();
ASSERT_EQ(Bindings.size(), 1);
const auto *BindingNode = static_cast<const Binding *>(Bindings[0].get());
ASSERT_TRUE(BindingNode);
const auto *InnerWith =
static_cast<const ExprWith *>(BindingNode->value().get());
ASSERT_TRUE(InnerWith);
ASSERT_EQ(InnerWith->kind(), Node::NK_ExprWith);

const auto *Var = static_cast<const ExprVar *>(InnerWith->expr());
ASSERT_TRUE(Var);
ASSERT_EQ(Var->kind(), Node::NK_ExprVar);

auto Scopes = VLA.getWithScopes(*Var);
ASSERT_EQ(Scopes.size(), 2);
// Innermost first
ASSERT_EQ(Scopes[0], InnerWith);
ASSERT_EQ(Scopes[1], &OuterWith);
}

TEST_F(VLATest, GetWithScopes_IndirectNested_AttrSet) {
// Indirect nested with through attrset: `with a; { y = with b; bar; }`
const char *Src = R"(with outer; { y = with inner; bar; })";
std::shared_ptr<Node> AST = parse(Src, Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_TRUE(AST);
const auto &OuterWith = *static_cast<const ExprWith *>(AST.get());

// Navigate to the inner with through the attrset
const auto *Attrs = static_cast<const ExprAttrs *>(OuterWith.expr());
ASSERT_TRUE(Attrs);
const auto *Binds = Attrs->binds();
ASSERT_TRUE(Binds);
const auto &Bindings = Binds->bindings();
ASSERT_EQ(Bindings.size(), 1);

const auto *BindingNode = static_cast<const Binding *>(Bindings[0].get());
ASSERT_TRUE(BindingNode);
const auto *InnerWith =
static_cast<const ExprWith *>(BindingNode->value().get());
ASSERT_TRUE(InnerWith);

const auto *Var = static_cast<const ExprVar *>(InnerWith->expr());
ASSERT_TRUE(Var);

auto Scopes = VLA.getWithScopes(*Var);
ASSERT_EQ(Scopes.size(), 2);
ASSERT_EQ(Scopes[0], InnerWith);
ASSERT_EQ(Scopes[1], &OuterWith);
}

TEST_F(VLATest, GetWithScopes_NoWith) {
// Variable not from with should return empty
const char *Src = R"(let x = 1; in x)";
std::shared_ptr<Node> AST = parse(Src, Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_TRUE(AST);
const auto *Let = static_cast<const ExprLet *>(AST.get());
const auto *Var = static_cast<const ExprVar *>(Let->expr());

auto Scopes = VLA.getWithScopes(*Var);
ASSERT_TRUE(Scopes.empty());
}

TEST_F(VLATest, GetWithScopes_TripleNested) {
// Triple nested with: variable should have 3 scopes
const char *Src = R"(with a; with b; with c; foo)";
std::shared_ptr<Node> AST = parse(Src, Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_TRUE(AST);
const auto &WithA = *static_cast<const ExprWith *>(AST.get());
const auto &WithB = *static_cast<const ExprWith *>(WithA.expr());
const auto &WithC = *static_cast<const ExprWith *>(WithB.expr());
const auto &Var = *static_cast<const ExprVar *>(WithC.expr());

auto Scopes = VLA.getWithScopes(Var);
ASSERT_EQ(Scopes.size(), 3);
// Innermost first
ASSERT_EQ(Scopes[0], &WithC);
ASSERT_EQ(Scopes[1], &WithB);
ASSERT_EQ(Scopes[2], &WithA);
}

} // namespace
6 changes: 6 additions & 0 deletions nixd/lib/Controller/CodeAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "CodeActions/JsonToNix.h"
#include "CodeActions/NoogleDoc.h"
#include "CodeActions/PackAttrs.h"
#include "CodeActions/WithToLet.h"

#include "nixd/Controller/Controller.h"

Expand Down Expand Up @@ -79,6 +80,11 @@ void Controller::onCodeAction(const lspserver::CodeActionParams &Params,
addExtractToFileAction(*N, *TU->parentMap(), *TU->variableLookup(),
FileURI, TU->src(), Actions);
}
// Add with-to-let action (requires VLA for variable tracking)
if (TU->variableLookup()) {
addWithToLetAction(*N, *TU->parentMap(), *TU->variableLookup(),
FileURI, TU->src(), Actions);
}
}
}

Expand Down
Loading