Also match argnames to validate methods (#108)

While working on TypedSyntax.jl it became apparent that CodeTracking
sometimes returns spurious results. At least some of these arise from
the recent support of anonymous functions, #102, which might in
retrospect have been ill-considered. Rather than back that change
out, this adopts a different resolution: validate the hits more
carefully.  The primary mechanism introduced here is to match not just
the function name, but also the argument names. This can work even for
anonymous functions, so we do not need to drop support for them.

This also adds quite a few new tests. These additions would have
passed before, but they proved valuable to ensure that the new
argname-matching works sufficiently well.

On TypedSyntax's "exhaustive.jl" test, this brings the
number of failed cases (specifically, the `badmis`) from
either 460 or 94 (depending on whether you include a few fixes
in TypedSyntax) to just 2.

Author: timholy

Project.toml

@@ -1,7 +1,7 @@
 name = "CodeTracking"
 uuid = "da1fd8a2-8d9e-5ec2-8556-3022fb5608a2"
 authors = ["Tim Holy <[email protected]>"]
-version = "1.2.2"
+version = "1.3.0"
 
 [deps]
 InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240"

README.md

@@ -143,3 +143,8 @@ file/line info in the method itself if Revise isn't running.)
 
 CodeTracking is perhaps best thought of as the "query" part of Revise.jl,
 providing a lightweight and stable API for gaining access to information it maintains internally.
+
+## Limitations (without Revise)
+
+- parsing sometimes starts on the wrong line. Line numbers are determined by counting `'\n'` in the source file, without parsing the contents. Consequently quoted- or in-code `'\n'` can mess up CodeTracking's notion of line numbering
+- default constructor methods for `struct`s are not found

src/CodeTracking.jl

@@ -243,24 +243,22 @@ function definition(::Type{String}, method::Method)
     src === nothing && return nothing
     src = replace(src, "\r"=>"")
     # Step forward to the definition of this method, keeping track of positions of newlines
+    # Issue: in-code `'\n'`. To fix, presumably we'd have to parse the entire file.
     eol = isequal('\n')
     linestarts = Int[]
     istart = 1
     for _ = 1:line-1
         push!(linestarts, istart)
         istart = findnext(eol, src, istart) + 1
     end
+    push!(linestarts, length(src) + 1)
     # Parse the function definition (hoping that we've found the right location to start)
     ex, iend = Meta.parse(src, istart; raise=false)
-    iend = prevind(src, iend)
-    if isfuncexpr(ex, methodname)
-        iend = min(iend, lastindex(src))
-        return clean_source(src[istart:iend]), line
-    end
-    # The function declaration was presumably on a previous line
+    # The function declaration may have been on a previous line,
+    # allow some slop
     lineindex = lastindex(linestarts)
     linestop = max(0, lineindex - 20)
-    while !isfuncexpr(ex, methodname) && lineindex > linestop
+    while !is_func_expr(ex, method) && lineindex > linestop
         istart = linestarts[lineindex]
         try
             ex, iend = Meta.parse(src, istart)
@@ -270,7 +268,7 @@ function definition(::Type{String}, method::Method)
         line -= 1
     end
     lineindex <= linestop && return nothing
-    return clean_source(src[istart:iend-1]), line
+    return clean_source(src[istart:prevind(src, iend)]), line
 end
 
 function clean_source(src)

src/utils.jl

@@ -1,12 +1,11 @@
 # This should stay as the first method because it's used in a test
 # (or change the test)
-function checkname(fdef::Expr, name)
-    fproto = fdef.args[1]
-    (fdef.head === :where || fdef.head == :(::)) && return checkname(fproto, name)
+function checkname(fdef::Expr, name)   # this is now unused
     fdef.head === :call || return false
+    fproto = fdef.args[1]
     if fproto isa Expr
-        fproto.head == :(::) && return last(fproto.args) == name
-        fproto.head == :curly && return fproto.args[1] === name
+        fproto.head == :(::) && return last(fproto.args) === name  # (obj::MyCallable)(x) = ...
+        fproto.head == :curly && return fproto.args[1] === name   # MyType{T}(x) = ...
         # A metaprogramming-generated function
         fproto.head === :$ && return true   # uncheckable, let's assume all is well
         # Is the check below redundant?
@@ -17,31 +16,128 @@ function checkname(fdef::Expr, name)
     isa(fproto, Symbol) || isa(fproto, QuoteNode) || isa(fproto, Expr) || return false
     return checkname(fproto, name)
 end
-checkname(fname::Symbol, name::Symbol) = begin
-    fname === name && return true
-    startswith(string(name), string('#', fname, '#')) && return true
-    string(name) == string(fname, "##kw") && return true
-    match(r"^#\d+$", string(name)) !== nothing && return true  # support `f = x -> 2x`
-    return false
+
+function get_call_expr(@nospecialize(ex))
+    while isa(ex, Expr) && ex.head ∈ (:where, :(::))
+        ex = ex.args[1]
+    end
+    isexpr(ex, :call) && return ex
+    return nothing
 end
-checkname(fname::Symbol, ::Nothing) = true
-checkname(fname::QuoteNode, name) = checkname(fname.value, name)
 
-function isfuncexpr(ex, name=nothing)
+function get_func_expr(@nospecialize(ex))
+    isa(ex, Expr) || return ex
     # Strip any macros that wrap the method definition
-    if ex isa Expr && ex.head === :toplevel
+    while isa(ex, Expr) && ex.head ∈ (:toplevel, :macrocall, :global, :local)
+        ex.head == :macrocall && length(ex.args) < 3 && return ex
         ex = ex.args[end]
     end
-    while ex isa Expr && ex.head === :macrocall && length(ex.args) >= 3
-        ex = ex.args[end]
+    isa(ex, Expr) || return ex
+    if ex.head == :(=) && length(ex.args) == 2
+        child1, child2 = ex.args
+        isexpr(get_call_expr(child1), :call) && return ex
+        isexpr(child2, :(->)) && return child2
     end
+    return ex
+end
+
+function is_func_expr(@nospecialize(ex))
     isa(ex, Expr) || return false
-    if ex.head === :function || ex.head === :(=)
-        return checkname(ex.args[1], name)
+    ex.head ∈ (:function, :(->)) && return true
+    if ex.head == :(=) && length(ex.args) == 2
+        child1 = ex.args[1]
+        isexpr(get_call_expr(child1), :call) && return true
     end
     return false
 end
 
+function is_func_expr(@nospecialize(ex), name::Symbol)
+    ex = get_func_expr(ex)
+    is_func_expr(ex) || return false
+    return checkname(get_call_expr(ex.args[1]), name)
+end
+
+function is_func_expr(@nospecialize(ex), meth::Method)
+    ex = get_func_expr(ex)
+    is_func_expr(ex) || return false
+    fname = nothing
+    if ex.head == :(->)
+        exargs = ex.args[1]
+        if isexpr(exargs, :tuple)
+            exargs = exargs.args
+        elseif (isa(exargs, Expr) && exargs.head ∈ (:(::), :.)) || isa(exargs, Symbol)
+            exargs = [exargs]
+        elseif isa(exargs, Expr)
+            return false
+        end
+    else
+        callex = get_call_expr(ex.args[1])
+        isexpr(callex, :call) || return false
+        fname = callex.args[1]
+        modified = true
+        while modified
+            modified = false
+            if isexpr(fname, :curly)    # where clause
+                fname = fname.args[1]
+                modified = true
+            end
+            if isexpr(fname, :., 2)        # module-qualified
+                fname = fname.args[2]
+                @assert isa(fname, QuoteNode)
+                fname = fname.value
+                modified = true
+            end
+            if isexpr(fname, :(::))
+                fname = fname.args[end]
+                modified = true
+            end
+        end
+        if !(isa(fname, Symbol) && is_gensym(fname)) && !isexpr(fname, :$)
+            if fname === :Type && isexpr(ex.args[1], :where) && isexpr(callex.args[1], :(::)) && isexpr(callex.args[1].args[end], :curly)
+                Tsym = callex.args[1].args[end].args[2]
+                for wheretyp in ex.args[1].args[2:end]
+                    @assert isexpr(wheretyp, :(<:))
+                    if Tsym == wheretyp.args[1]
+                        fname = wheretyp.args[2]
+                        break
+                    end
+                end
+            end
+            # match the function name
+            fname === strip_gensym(meth.name) || return false
+        end
+        exargs = callex.args[2:end]
+    end
+    # match the argnames
+    if !isempty(exargs) && isexpr(first(exargs), :parameters)
+        popfirst!(exargs)   # don't match kwargs
+    end
+    margs = Base.method_argnames(meth)
+    _, idx = kwmethod_basename(meth)
+    if idx > 0
+        margs = margs[idx:end]
+    end
+    for (arg, marg) in zip(exargs, margs[2:end])
+        aname = get_argname(arg)
+        aname === :_ && continue
+        aname === marg || (aname === Symbol("#unused#") && marg === Symbol("")) || return false
+    end
+    return true  # this will match any fcn `() -> ...`, but file/line is the only thing we have
+end
+
+function get_argname(@nospecialize(ex))
+    isa(ex, Symbol) && return ex
+    isexpr(ex, :(::), 2) && return get_argname(ex.args[1])      # type-asserted
+    isexpr(ex, :(::), 1) && return Symbol("#unused#") # nameless args (e.g., `::Type{String}`)
+    isexpr(ex, :kw) && return get_argname(ex.args[1])           # default value
+    isexpr(ex, :(=)) && return get_argname(ex.args[1])          # default value inside `@nospecialize`
+    isexpr(ex, :macrocall) && return get_argname(ex.args[end])  # @nospecialize
+    isexpr(ex, :...) && return get_argname(only(ex.args))       # varargs
+    isexpr(ex, :tuple) && return Symbol("")                     # tuple-destructuring
+    dump(ex)
+    error("unexpected argument ", ex)
+end
+
 function linerange(def::Expr)
     start, haslinestart = findline(def, identity)
     stop, haslinestop  = findline(def, Iterators.reverse)
@@ -70,6 +166,72 @@ Base.convert(::Type{LineNumberNode}, lin::LineInfoNode) = LineNumberNode(lin.lin
 
 # This regex matches the pseudo-file name of a REPL history entry.
 const rREPL = r"^REPL\[(\d+)\]$"
+# Match anonymous function names
+const rexfanon = r"^#\d+$"
+# Match kwfunc method names
+const rexkwfunc = r"^#.*##kw$"
+
+is_gensym(s::Symbol) = is_gensym(string(s))
+is_gensym(str::AbstractString) = startswith(str, '#')
+
+strip_gensym(s::Symbol) = strip_gensym(string(s))
+function strip_gensym(str::AbstractString)
+    if startswith(str, '#')
+        idx = findnext('#', str, 2)
+        if idx !== nothing
+            return Symbol(str[2:idx-1])
+        end
+    end
+    endswith(str, "##kw") && return Symbol(str[1:end-4])
+    return Symbol(str)
+end
+
+if isdefined(Core, :kwcall)
+    is_kw_call(m::Method) = Base.unwrap_unionall(m.sig).parameters[1] === typeof(Core.kwcall)
+else
+    function is_kw_call(m::Method)
+        T = Base.unwrap_unionall(m.sig).parameters[1]
+        return match(rexkwfunc, string(T.name.name)) !== nothing
+    end
+end
+
+# is_body_fcn(m::Method, basename::Symbol) = match(Regex("^#$basename#\\d+\$"), string(m.name)) !== nothing
+# function is_body_fcn(m::Method, basename::Expr)
+#     basename.head == :. || return false
+#     return is_body_fcn(m, get_basename(basename))
+# end
+# is_body_fcn(m::Method, ::Nothing) = false
+# function get_basename(basename::Expr)
+#     bn = basename.args[end]
+#     @assert isa(bn, QuoteNode)
+#     return is_body_fcn(m, bn.value)
+# end
+
+function kwmethod_basename(meth::Method)
+    name = meth.name
+    sname = string(name)
+    mtch = match(r"^(.*)##kw$", sname)
+    if mtch === nothing
+        mtch = match(r"^#+(.*)#", sname)
+    end
+    name = mtch === nothing ? name : Symbol(only(mtch.captures))
+    ftypname = Symbol(string('#', name))
+    idx = findfirst(Base.unwrap_unionall(meth.sig).parameters) do @nospecialize(T)
+        if isa(T, DataType)
+            Tname = T.name.name
+            if Tname === :Type
+                p1 = Base.unwrap_unionall(T.parameters[1])
+                Tname = isa(p1, DataType) ? p1.name.name :
+                        isa(p1, TypeVar) ? p1.name : error("unexpected type ", typeof(p1), "for ", meth)
+                return Tname == name
+            end
+            return ftypname === Tname
+        end
+        false
+    end
+    idx === nothing && return name, 0
+    return name, idx
+end
 
 """
     src = src_from_file_or_REPL(origin::AbstractString, repl = Base.active_repl)