Merge pull request #402 from gcv/fix-relative-imports

Fix relative import resolution

Author: pfitzseb

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)

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)

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

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
 
 """

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