various refactors

- add TODO comment to refactor module-detection strategy
- add tests for current module-detection approaches
- improve utility tests

Author: aviatesk

src/goto.jl

@@ -165,10 +165,6 @@ function searchtoplevelitems(mod::Module, text::String, path::Nothing)
   return pathitemsmaps
 end
 
-# TODO:
-# use the module detection logic below for general module auto-detection,
-# e.g.: `module` handler and such
-
 # sub entry method
 function _searchtoplevelitems(mod::Module, pathitemsmaps::PathItemsMaps)
   entrypath, paths = modulefiles(mod) # Revise-like approach

src/refactor.jl

@@ -166,7 +166,7 @@ function _globalrenamerefactor(old, new, mod, expr, files)
       if ex === oldsym
         push!(modifiedfiles, fullpath(file))
         newsym
-      # handle dot (module) accessor
+      # handle dot accessor
       elseif @capture(ex, m_.$oldsym) && getfield′(mod, Symbol(m)) isa Module
         push!(modifiedfiles, fullpath(file))
         Expr(:., m, newsym)

src/utils.jl

@@ -230,13 +230,36 @@ urigoto(mod, word) = "atom://julia-client/?goto=true&mod=$(mod)&word=$(word)"
 urimoduleinfo(mod) = "atom://julia-client/?moduleinfo=true&mod=$(mod)"
 
 #=
-module file detections
+utilities to find module files
 
 adapted from https://github.com/timholy/Revise.jl/tree/b0c5c864ea78b93caaa820cb9cfc45eca47f43ff
+
+# TODO:
+# use the module detection logic below for general module auto-detection instead of CodeTools.jl
+# e.g.: `module` handler and such
 =#
 
 using Base: PkgId, UUID
 
+"""
+    entrypath, line = moduledefinition(mod::Module)
+
+Returns an entry file of `mod`, and its definition line.
+
+!!! note
+    This function works for non-precompiled packages.
+"""
+function moduledefinition(mod::Module) # NOTE: added when adapted
+  evalmethod = first(methods(getfield(mod, :eval)))
+  parentfile = String(evalmethod.file)
+  line = evalmethod.line
+  id = Base.PkgId(mod)
+  if id.name == "Base" || id.name == "Core" || Symbol(id.name) ∈ stdlib_names  # NOTE: "Core" is added when adapted
+    parentfile = normpath(Base.find_source_file(parentfile))
+  end
+  fixpath(parentfile), line
+end
+
 """
     parentfile, included_files = modulefiles(mod::Module)
 
@@ -261,13 +284,16 @@ function modulefiles(mod::Module)
   return fixpath(parentfile), [fixpath(mf[2]) for mf in included_files]
 end
 
+# TODO: excludes files in submodules
+# TODO: looks for non-toplevel `include` calls
 """
     included_files = modulefiles(entrypath::String)::Vector{String}
 
-Return all the file paths that can be reached via [`include`](@ref) calls.
-Note this function currently only looks for static _toplevel_ calls.
+Returns all the files that can be reached via [`include`](@ref) calls from `entrypath`.
+Note this function currently only looks for static toplevel calls (i.e. miss the calls
+  in not in toplevel scope), and can include files in the submodules as well.
 """
-function modulefiles(entrypath::String, files = [])
+function modulefiles(entrypath::String, files = Vector{String}())
   isfile′(entrypath) || return files
 
   push!(files, entrypath)
@@ -291,17 +317,6 @@ function modulefiles(entrypath::String, files = [])
   return files
 end
 
-function moduledefinition(mod::Module) # NOTE: added when adapted
-  evalmethod = first(methods(getfield(mod, :eval)))
-  parentfile = String(evalmethod.file)
-  line = evalmethod.line
-  id = Base.PkgId(mod)
-  if id.name == "Base" || id.name == "Core" || Symbol(id.name) ∈ stdlib_names  # NOTE: "Core" is added when adapted
-    parentfile = normpath(Base.find_source_file(parentfile))
-  end
-  fixpath(parentfile), line
-end
-
 # Fix paths to files that define Julia (base and stdlibs)
 function fixpath(
   filename::AbstractString;

test/goto.jl

@@ -1,5 +1,5 @@
 @testset "goto symbols" begin
-    using Atom: modulegotoitems, realpath′, toplevelgotoitems, SYMBOLSCACHE,
+    using Atom: modulegotoitems, toplevelgotoitems, SYMBOLSCACHE,
                 regeneratesymbols, methodgotoitems, globalgotoitems
     using CSTParser
 
@@ -51,19 +51,18 @@
 
     @testset "module goto" begin
         let item = modulegotoitems("Atom", Main)[1]
-            @test item.file == realpath′(joinpath(@__DIR__, "..", "src", "Atom.jl"))
+            @test item.file == joinpath′(atomjldir, "Atom.jl")
             @test item.line == 3
         end
         let item = modulegotoitems("Junk2", Main.Junk)[1]
-            @test item.file == joinpath(@__DIR__, "fixtures", "Junk.jl")
+            @test item.file == joinpath′(junkpath)
             @test item.line == 14
         end
     end
 
     @testset "goto toplevel symbols" begin
         ## where Revise approach works, i.e.: precompiled modules
-        let dir = realpath′(joinpath(@__DIR__, "..", "src"))
-            path = joinpath(dir, "comm.jl")
+        let path = joinpath′(atomjldir, "comm.jl")
             text = read(path, String)
             mod = Atom
             key = "Atom"
@@ -79,23 +78,8 @@
             # check caching works
             @test haskey(SYMBOLSCACHE, key)
 
-            # check the Revise-like approach finds all the included files
-            let numfiles = 0
-                debuggerpath = realpath′(joinpath(@__DIR__, "..", "src", "debugger"))
-                profilerpath = realpath′(joinpath(@__DIR__, "..", "src", "profiler"))
-                for (d, ds, fs) ∈ walkdir(dir)
-                    if d ∈ (debuggerpath, profilerpath)
-                        numfiles += 1 # debugger.jl / traceur.jl (in Atom module)
-                        continue
-                    end
-                    for f ∈ fs
-                        if endswith(f, ".jl") # .jl check is needed for travis, who creates hoge.cov files
-                            numfiles += 1
-                        end
-                    end
-                end
-                @test length(SYMBOLSCACHE[key]) == numfiles
-            end
+            # check the Revise-like approach finds all files in Atom module
+            @test length(SYMBOLSCACHE[key]) == length(atommodfiles)
 
             # when `path` isn't given, i.e. via docpane / workspace
             let items = toplevelgotoitems(word, mod, "", nothing) .|> Dict
@@ -106,11 +90,18 @@
 
             # same as above, but without any previous cache -- falls back to CSTPraser-based module-walk
             delete!(SYMBOLSCACHE, key)
+
             let items = toplevelgotoitems(word, mod, "", nothing) .|> Dict
                 @test !isempty(items)
                 @test items[1][:file] == path
                 @test items[1][:text] == word
             end
+
+            # check CSTPraser-based module-walk finds all the included files
+            # currently broken:
+            # - files in submodules are included
+            # - webio.jl is excluded since `include("webio.jl")` is a toplevel call
+            @test_broken length(SYMBOLSCACHE[key]) == length(atommoddir)
         end
 
         ## where the Revise-like approach doesn't work, e.g. non-precompiled modules
@@ -143,15 +134,15 @@
         # handle dot accessors gracefully
         let
             # can access the non-exported (non-method) bindings in the other module
-            path = realpath′(joinpath(@__DIR__, "..", "src", "goto.jl"))
+            path = joinpath′(@__DIR__, "..", "src", "goto.jl")
             text = read(@__FILE__, String)
             items = Dict.(toplevelgotoitems("Atom.SYMBOLSCACHE", Main, text, @__FILE__))
             @test !isempty(items)
             @test items[1][:file] == path
             @test items[1][:text] == "SYMBOLSCACHE"
 
             # handle if a module is duplicated
-            path = realpath′(joinpath(@__DIR__, "..", "src", "comm.jl"))
+            path = joinpath′(@__DIR__, "..", "src", "comm.jl")
             text = read(path, String)
             items = Dict.(toplevelgotoitems("Atom.handlers", Atom, text, path))
             @test !isempty(items)

test/runtests.jl

@@ -4,14 +4,50 @@ using Lazy
 import JSON
 
 
+joinpath′(files...) = Atom.fullpath(joinpath(files...))
+
+atomjldir = joinpath′(@__DIR__, "..", "src")
+
+webiofile = joinpath′(atomjldir, "display", "webio.jl")
+
+# files in `Atom` module (except files in its submodules)
+atommodfiles = let
+    files = []
+    debuggerdir = joinpath′(atomjldir, "debugger")
+    profilerdir = joinpath′(atomjldir, "profiler")
+    for (d, ds, fs) in walkdir(atomjldir)
+        # NOTE: update directories below when you create an new submodule
+        # the 2 files below are in Atom module
+        if d == debuggerdir
+            push!(files, joinpath′(d, "debugger.jl"))
+            continue
+        end
+        if d == profilerdir
+            push!(files, joinpath′(d, "profiler.jl"))
+            push!(files, joinpath′(d, "traceur.jl"))
+            continue
+        end
+        for f in fs
+            # NOTE: currently both Revise-like and CSTPraser-based approach fails
+            # to detect display/webio.jl as a file in Atom module
+            f == "webio.jl" && continue
+
+            # .jl check is needed for travis, who creates hoge.cov files
+            endswith(f, ".jl") && push!(files, joinpath′(d, f))
+        end
+    end
+    files
+end
+
 # mock a listener
 Core.eval(Atom, Meta.parse("sock = IOBuffer()"))
 readmsg() = JSON.parse(String(take!(Atom.sock)))
 
 # mock Module
-junkpath = joinpath(@__DIR__, "fixtures", "Junk.jl")
+junkpath = joinpath′(@__DIR__, "fixtures", "Junk.jl")
 include(junkpath)
 
+
 include("./misc.jl") # basics
 include("./utils.jl")
 include("./eval.jl")

test/utils.jl

@@ -48,7 +48,7 @@ end
     @test Atom.finddevpackages() isa AbstractDict
 end
 
-#TODO: baselink, edit
+# TODO: baselink, edit
 
 cd(old_pwd)
 end
@@ -79,18 +79,80 @@ end
     end
 end
 
-@testset "limiting excessive strings" begin
-    using Atom: strlimit
+@testset "is utilities" begin
+    @testset "iskeyword" begin
+        using Atom: iskeyword
 
-    # only including ASCII
-    @test strlimit("julia", 5) == "julia"
-    @test strlimit("julia", 4) == "jul…"
-    @test strlimit("Julia in the Nutshell", 21, " ...") == "Julia in the Nutshell"
-    @test strlimit("Julia in the Nutshell", 20, " ...") == "Julia in the Nut ..."
+        @test iskeyword(:begin)
+        @test iskeyword("begin")
+        @test !iskeyword(:iskeyword)
+    end
+end
+
+@testset "string utilities" begin
+    @testset "limiting excessive strings" begin
+        using Atom: strlimit
+
+        # only including ASCII
+        @test strlimit("julia", 5) == "julia"
+        @test strlimit("julia", 4) == "jul…"
+        @test strlimit("Julia in the Nutshell", 21, " ...") == "Julia in the Nutshell"
+        @test strlimit("Julia in the Nutshell", 20, " ...") == "Julia in the Nut ..."
+
+        # including Unicode: should respect _length_ of strings, not code units
+        @test strlimit("jμλια", 5) == "jμλια"
+        @test strlimit("jμλια", 4) == "jμλ…"
+        @test strlimit("Jμλια in the Nutshell", 21, " ...") == "Jμλια in the Nutshell"
+        @test strlimit("Jμλια in the Nutshell", 20, " ...") == "Jμλια in the Nut ..."
+    end
+end
 
-    # including Unicode: should respect _length_ of strings, not code units
-    @test strlimit("jμλια", 5) == "jμλια"
-    @test strlimit("jμλια", 4) == "jμλ…"
-    @test strlimit("Jμλια in the Nutshell", 21, " ...") == "Jμλια in the Nutshell"
-    @test strlimit("Jμλια in the Nutshell", 20, " ...") == "Jμλια in the Nut ..."
+@testset "module utilties" begin
+    import Atom: moduledefinition, modulefiles
+
+    @testset "find module definiton location" begin
+        let (path, line) = moduledefinition(Atom)
+            @test path == joinpath′(@__DIR__, "..", "src", "Atom.jl")
+            @test line == 4
+        end
+        let (path, line) = moduledefinition(Junk)
+            @test path == joinpath′(@__DIR__, "fixtures", "Junk.jl")
+            @test line == 1
+        end
+        let (path, line) = moduledefinition(Junk.Junk2)
+            @test path == joinpath′(@__DIR__, "fixtures", "Junk.jl")
+            @test line == 15
+        end
+    end
+
+    @testset "find module files" begin
+        ## Revise-like module file detection
+        # works for precompiled packages
+        let (parentfile, included_files) = modulefiles(Atom)
+            expected = Set(atommodfiles)
+            actual = Set((parentfile, included_files...))
+            @test actual == expected
+
+            # can't detect display/webio.jl
+            @test_broken webiofile in modfiles
+        end
+
+        # fails for non-precompiled packages
+        @test_broken junkpath == modulefiles(Junk)[1]
+
+        ## CSTPraser-based module file detection
+        let included_files = normpath.(modulefiles(joinpath′(atomjldir, "Atom.jl")))
+            # finds all the files in Atom module except display/webio.jl
+            for f in atommodfiles
+                f == webiofile && continue
+                @test f in included_files
+            end
+
+            # can't exclude files in the submodules
+            @test_broken length(atommodfiles) == length(included_files)
+
+            # can't look for non-toplevel `include` calls
+            @test_broken webiofile in included_files
+        end
+    end
 end