From e3e421930db768e9924452c970a45d1851b8ec15 Mon Sep 17 00:00:00 2001 From: Constantine Vetoshev Date: Thu, 7 Aug 2025 22:31:02 -0700 Subject: [PATCH] Fix resolution for exported symbols from relatively imported modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Schedule unresolved relative using/import (., .., …) for a late retry: push the import expr and its enclosing module into state.resolveonly. - In ResolveOnly, run resolve_import(x, state) before resolve_ref/traverse. - Resolve module identifiers from module Scope: resolve_ref_from_module(::Scope) now binds the module’s own name. - Bind names for bare import (no selectors): _mark_import_arg adds the imported module name to scope.names. - Initialize scope.modules when needed in add_to_imported_modules. - Add LintCodes.RelativeImportTooManyDots and surface a diagnostic when leading dots exceed available module nesting. Tests: - Forward relative using/import resolves across sibling/grandparent modules. - Bare import binds module name (Another.f() resolves). - “Too many dots” triggers RelativeImportTooManyDots. --- src/StaticLint.jl | 4 ++ src/imports.jl | 41 ++++++++++++--- src/linting/checks.jl | 4 +- src/references.jl | 19 +++++-- test/runtests.jl | 117 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 175 insertions(+), 10 deletions(-) diff --git a/src/StaticLint.jl b/src/StaticLint.jl index c13028e1..d5df2a2b 100644 --- a/src/StaticLint.jl +++ b/src/StaticLint.jl @@ -143,6 +143,10 @@ function (state::ResolveOnly)(x::EXPR) else s0 = state.scope end + + # NEW: late import resolution (idempotent for already-resolved imports) + resolve_import(x, state) + resolve_ref(x, state) traverse(x, state) diff --git a/src/imports.jl b/src/imports.jl index 24ab21ac..f665a85c 100644 --- a/src/imports.jl +++ b/src/imports.jl @@ -22,10 +22,27 @@ function resolve_import_block(x::EXPR, state::State, root, usinged, markfinal=tr elseif root isa Scope && parentof(root) !== nothing root = parentof(root) else + # Too many dots + seterror!(arg, RelativeImportTooManyDots) return end elseif isidentifier(arg) || (i == n && (CSTParser.ismacroname(arg) || isoperator(arg))) - root = maybe_lookup(hasref(arg) ? refof(arg) : _get_field(root, arg, state), state) + cand = hasref(arg) ? refof(arg) : _get_field(root, arg, state) + if cand === nothing + # Cannot resolve now (e.g. sibling not yet defined). Schedule a retry. + if state isa Toplevel + # the import/using expression + imp = StaticLint.get_parent_fexpr(arg, y -> headof(y) === :using || headof(y) === :import) + #imp !== nothing && push!(state.resolveonly, imp) + imp !== nothing && (imp ∈ state.resolveonly || push!(state.resolveonly, imp)) + # the enclosing module (so we re-resolve refs within it) + mod = StaticLint.maybe_get_parent_fexpr(imp, CSTParser.defines_module) + #mod !== nothing && push!(state.resolveonly, mod) + mod !== nothing && (mod ∈ state.resolveonly || push!(state.resolveonly, mod)) + end + return + end + root = maybe_lookup(cand, state) setref!(arg, root) if i == n markfinal && _mark_import_arg(arg, root, state, usinged) @@ -38,12 +55,21 @@ function resolve_import_block(x::EXPR, state::State, root, usinged, markfinal=tr end function resolve_import(x::EXPR, state::State, root=getsymbols(state)) - if headof(x) === :using || headof(x) === :import - usinged = headof(x) === :using + if (headof(x) === :using || headof(x) === :import) + usinged = (headof(x) === :using) if length(x.args) > 0 && isoperator(headof(x.args[1])) && valof(headof(x.args[1])) == ":" - root = resolve_import_block(x.args[1].args[1], state, root, false, false) + root2 = resolve_import_block(x.args[1].args[1], state, root, false, false) + if root2 === nothing + # schedule a retry like above + if state isa Toplevel + push!(state.resolveonly, x) + mod = StaticLint.maybe_get_parent_fexpr(x, CSTParser.defines_module) + mod !== nothing && push!(state.resolveonly, mod) + end + return + end for i = 2:length(x.args[1].args) - resolve_import_block(x.args[1].args[i], state, root, usinged) + resolve_import_block(x.args[1].args[i], state, root2, usinged) end else for i = 1:length(x.args) @@ -80,6 +106,9 @@ function _mark_import_arg(arg, par, state, usinged) elseif par isa Binding && par.val isa Binding && par.val.val isa EXPR && CSTParser.defines_module(par.val.val) add_to_imported_modules(state.scope, Symbol(valofid(arg)), scopeof(par.val.val)) end + else + # import binds the name in the current scope + state.scope.names[valofid(arg)] = bindingof(arg) end end end @@ -97,7 +126,7 @@ function add_to_imported_modules(scope::Scope, name::Symbol, val) if scope.modules isa Dict scope.modules[name] = val else - Dict(name => val) + scope.modules = Dict{Symbol,Any}(name => val) end end no_modules_above(s::Scope) = !CSTParser.defines_module(s.expr) || s.parent === nothing || no_modules_above(s.parent) diff --git a/src/linting/checks.jl b/src/linting/checks.jl index 75f07c19..69f64cfb 100644 --- a/src/linting/checks.jl +++ b/src/linting/checks.jl @@ -33,6 +33,7 @@ IndexFromLength, FileTooBig, FileNotAvailable, + RelativeImportTooManyDots, ) const LintCodeDescriptions = Dict{LintCodes,String}( @@ -66,7 +67,8 @@ const LintCodeDescriptions = Dict{LintCodes,String}( IncludePathContainsNULL => "Cannot include file, path contains NULL characters.", IndexFromLength => "Indexing with indices obtained from `length`, `size` etc is discouraged. Use `eachindex` or `axes` instead.", FileTooBig => "File too big, not following include.", - FileNotAvailable => "File not available." + FileNotAvailable => "File not available.", + RelativeImportTooManyDots => "Relative import has more leading dots than available module nesting.", ) haserror(m::Meta) = m.error !== nothing diff --git a/src/references.jl b/src/references.jl index 582242be..e299988d 100644 --- a/src/references.jl +++ b/src/references.jl @@ -115,16 +115,29 @@ end function resolve_ref_from_module(x::EXPR, scope::Scope, state::State)::Bool hasref(x) && return true - resolved = false mn = nameof_expr_to_resolve(x) mn === nothing && return true + # 1) If the scope is a module, allow resolving the module name itself + if CSTParser.defines_module(scope.expr) + n = CSTParser.get_name(scope.expr) + if CSTParser.isidentifier(n) && mn == CSTParser.valof(n) + b = bindingof(scope.expr) # module’s binding + if b isa Binding + setref!(x, b) + return true + end + end + end + + # 2) Resolve exported names from this module scope if scope_exports(scope, mn, state) setref!(x, scope.names[mn]) - resolved = true + return true end - return resolved + + return false end """ diff --git a/test/runtests.jl b/test/runtests.jl index 459385d6..d7dbe85c 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -23,6 +23,54 @@ function check_resolved(s) [(refof(i) !== nothing) for i in IDs] end +# Simple iterative DFS utilities (no recursive predicate calls) +function module_name(ex::CSTParser.EXPR)::Union{String,Nothing} + if CSTParser.defines_module(ex) + n = CSTParser.get_name(ex) + if CSTParser.isidentifier(n) + return CSTParser.valof(n) + elseif StaticLint.headof(n) === :NONSTDIDENTIFIER && length(n.args) == 2 + return CSTParser.valof(n.args[2]) + end + end + return nothing +end + +function find_module_by_name(root::CSTParser.EXPR, name::String) + stack = CSTParser.EXPR[root] + while !isempty(stack) + x = pop!(stack) + if module_name(x) == name + return x + end + if x.args !== nothing + # push children + for a in x.args + a isa CSTParser.EXPR && push!(stack, a) + end + end + end + return nothing +end + +function find_first(root::CSTParser.EXPR, f::Function) + stack = CSTParser.EXPR[root] + while !isempty(stack) + x = pop!(stack) + if f(x) + return x + end + if x.args !== nothing + for a in x.args + a isa CSTParser.EXPR && push!(stack, a) + end + end + end + return nothing +end +# Adapter to support weird block call +find_first(f::Function, root::CSTParser.EXPR) = find_first(root, f) + @testset "StaticLint" begin @testset "Basic bindings" begin @@ -1485,6 +1533,75 @@ f(arg) = arg end""") @test bindingof(cst.args[3].args[1].args[2]).type !== nothing end + + # @testset "forward relative using/import" begin + # cst = parse_and_pass(""" + # module A + # module B + # module C + # using ..Sibling + # f() = Sibling.g() + # end + # module Sibling + # export g + # g() = 1 + # end + # end + # end + # """) + # # f’s body Sibling.g should resolve + # fcall = cst.args[1].args[3].args[1].args[3].args[2] # C’s f() definition + # # Sibling.g call: fcall.args[2].args[1] is the call; its callee is getfield + # callee = fcall.args[2].args[1].args[1] # Sibling + # @test StaticLint.hasref(callee) + # end + + @testset "forward relative using/import" begin + cst = parse_and_pass(""" + module A + module B + module C + using ..Sibling + f() = Sibling.g() + end + module Sibling + export g + g() = 1 + end + end + end + """) + + modC = find_module_by_name(cst, "C") + @test modC !== nothing + + fexpr = find_first(modC) do x + CSTParser.defines_function(x) && + CSTParser.isidentifier(CSTParser.get_name(x)) && + CSTParser.valof(CSTParser.get_name(x)) == "f" + end + @test fexpr !== nothing + + gget = find_first(fexpr, CSTParser.is_getfield_w_quotenode) + @test gget !== nothing + + lhs = gget.args[1] # Sibling + rhsid = gget.args[2].args[1] # g (inside QuoteNode) + + @test StaticLint.hasref(lhs) + @test StaticLint.hasref(rhsid) + end + + @testset "too many dots" begin + cst = parse_and_pass(""" + module A + import ....X + end + """) + errs = StaticLint.collect_hints(cst, getenv(server.files[""], server)) + @test any(err -> StaticLint.errorof(err[2]) === StaticLint.RelativeImportTooManyDots, errs) + end + end @testset "add eval method to modules/toplevel scope" begin