Fix import-before-module check

base/loading.jl

@@ -2792,33 +2792,39 @@
     return code
 end
 
+# Return true if module declaration is found
+# Throw an error if using/import is found
+# Otherwise return false
+function _check_src_module_wrap(pkg::PkgId, srcpath::String, ex::Expr)
+    isexpr(ex, [:using, :import]) && throw(ErrorException("Package $(repr("text/plain", pkg)): file $srcpath has a using/import before a module declaration."))
+    isexpr(ex, :module) && return true
+    for arg in ex.args
+        if isa(arg, Expr)
+            _check_src_module_wrap(pkg, srcpath, arg) && return true
+        end
+    end
+    return false
+end
+
 """
     check_src_module_wrap(srcpath::String)
 
 Checks that a package entry file `srcpath` has a module declaration, and that it is before any using/import statements.
 """
 function check_src_module_wrap(pkg::PkgId, srcpath::String)
-    module_rgx = r"^(|end |\"\"\" )\s*(?:@)*(?:bare)?module\s"
-    load_rgx = r"\b(?:using|import)\s"
-    load_seen = false
-    inside_string = false
-    for s in eachline(srcpath)
-        if count("\"\"\"", s) == 1
-            # ignore module docstrings
-            inside_string = !inside_string
-        end
-        inside_string && continue
-        if contains(s, module_rgx)
-            if load_seen
-                throw(ErrorException("Package $(repr("text/plain", pkg)) source file $srcpath has a using/import before a module declaration."))
-            end
-            return true
-        end
-        if startswith(s, load_rgx)
-            load_seen = true
-        end
-    end
-    throw(ErrorException("Package $(repr("text/plain", pkg)) source file $srcpath does not contain a module declaration."))
+    # Fast path: if the source file starts with a module declaration, just
+    # return true
+    open(startswith("module "), srcpath) && return true
+
+    source = read(srcpath, String)
+    ex, pos = Meta.parse(source, 1)
+    while ex !== nothing
+        # Dosctrings are parsed as an expression with the module definition in
+        # the expression's argument, so we need recursive check
+        _check_src_module_wrap(pkg, srcpath, ex) && return true
+        ex, pos = Meta.parse(source, pos)
+    end
+    throw(ErrorException("Package $(repr("text/plain", pkg)): file $srcpath does not contain a module declaration."))
 end
 
 # this is called in the external process that generates precompiled package files

test/loading.jl

@@ -1271,22 +1271,26 @@ end
 @testset "checking srcpath modules" begin
     p = Base.PkgId("Dummy")
     fpath, _ = mktemp()
+
+    function check(src)
+        write(fpath, src)
+        return Base.check_src_module_wrap(p, fpath)
+    end
+
     @testset "valid" begin
-        write(fpath, """
+        @test check("""
         module Foo
         using Bar
         end
         """)
-        @test Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test check("""
         baremodule Foo
         using Bar
         end
         """)
-        @test Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test check("""
         \"\"\"
         Foo
         using Foo
@@ -1295,66 +1299,112 @@ end
         using Bar
         end
         """)
-        @test Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test check("""
         \"\"\" Foo \"\"\"
         module Foo
         using Bar
         end
         """)
-        @test Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test check("""
         \"\"\"
         Foo
         \"\"\" module Foo
         using Bar
         end
         """)
-        @test Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test check("""
         @doc let x = 1
             x
         end module Foo
         using Bar
         end
         """)
-        @test Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test check("""
         # using foo
         module Foo
         using Bar
         end
         """)
-        @test Base.check_src_module_wrap(p, fpath)
+
+        @test check("""
+        #=
+        using foo
+        =#
+        module Foo
+        using Bar
+        end
+        """)
+
+        @test check("""
+        #=
+        nested multiline comment
+        #=
+        using foo
+        =#
+        =#
+        module Foo
+        using Bar
+        end
+        """)
+
+        @test check("""
+        #= using foo =#
+        module Foo
+        using Bar
+        end
+        """)
+
+        @test check("""
+        #= \"\"\" =#
+        module Foo
+        #= \"\"\" =#
+        using Bar
+        end
+        """)
     end
     @testset "invalid" begin
-        write(fpath, """
+        @test_throws ErrorException check("""
         # module Foo
         using Bar
         # end
         """)
-        @test_throws ErrorException Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test_throws ErrorException check("""
         using Bar
         module Foo
         end
         """)
-        @test_throws ErrorException Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test_throws ErrorException check("""
         using Bar
         """)
-        @test_throws ErrorException Base.check_src_module_wrap(p, fpath)
 
-        write(fpath, """
+        @test_throws ErrorException check("""
         x = 1
         """)
-        @test_throws ErrorException Base.check_src_module_wrap(p, fpath)
+
+        @test_throws ErrorException check("""
+        using Bar #=
+        =#
+        module Foo
+        end
+        """)
+
+        @test_throws ErrorException check("""
+        #=
+        module Foo
+        =#
+        using Bar
+        module Foo2
+        end
+        #=
+        end
+        =#
+        """)
     end
 end