Merge pull request #184 from TotalVerb/fw/fix-180

Fix #180 by enhancing static type analysis

REQUIRE

@@ -1,3 +1,3 @@
 julia 0.5
 JSON 0.6.0
-Compat 0.15.0
+Compat 0.20.0

docs/features.md

@@ -39,7 +39,6 @@
   `@lintpragma("Ignore unstable type variable [variable name]")` just before the warning.
 * Incompatible type assertion and assignment e.g. `a::Int = 1.0`
 * Incompatible tuple assignment sizes e.g. `(a,b) = (1,2,3)`
-* Flatten behavior of nested vcat e.g. `[[1,2],[3,4]]`
 * Loop over a single number. e.g. `for i=1 end`
 * More indices than dimensions in an array lookup
 * Look up a dictionary with the wrong key type, if the key's type can be inferred.

docs/messages.md

@@ -86,27 +86,30 @@ Every error code starts with letter for the severity `E`:`ERROR`, `W`:`WARN` or
 | E434   | value at position #i is the referenced x. Possible typo?
 | E435   | new is provided with more arguments than fields
 | E436   | more indices than dimensions
+| E437   | @compat called with wrong number of arguments
 |        |
 | **E5** | *Type Error*
 | E511   | apparent non-Bool type
 | E512   | lint doesn't understand expresion in a boolean context
 | E513   | leaf type as a type constraint it makes no sense
 | E516   | type assertion and default seem inconsistent
 | E517   | constructor-like function name doesn't match type T
-| E518   | key type expects X, provided Y
-| E519   | string[] expects Integer, provided X
-| E521   | apparent type T is not a container type
+| E518   | (merged into E522)
+| E519   | (merged into E522)
+| E521   | (merged into E522)
+| E522   | indexing T with index types [S, U] is not supported
 | E523   | constructor parameter collides with a type parameter
 | E524   | bitstype needs its 2nd argument to be a new type symbol
 | E525   | is of an immutable type
 | E531   | multiple key types detected. Use Dict{Any,V}() for mixed type dict
 | E532   | multiple value types detected. Use Dict{K,Any}() for mixed type dict
 | E533   | type parameters are invariant, try f{T<:Number}(x::T)...
 | E534   | introducing a new name for an implicit argument to the function, use {T<:X}
-| E535   | introducing a new name for an algebric data type, use {T<:X}
+| E535   | (no longer used)
 | E536   | use {T<:...} instead of a known type
 | E537   | (removed)
 | E538   | known type in parametric data type, use {T<:...}
+| E539   | assigning an error to a variable
 |        |
 | **E6** | *Structure Error*
 | E611   | constructor doesn't seem to return the constructed object
@@ -140,6 +143,7 @@ Every error code starts with letter for the severity `E`:`ERROR`, `W`:`WARN` or
 | W543   | cannot determine if Type or not
 | W544   | cannot determine if Type or not
 | W545   | previously used variable has apparent type X, but now assigned Y
+| W546   | implicitly discarding values, m of n used
 |        |
 | **W6** | *Structure Warning*
 | W641   | unreachable code after return
@@ -163,6 +167,7 @@ Every error code starts with letter for the severity `E`:`ERROR`, `W`:`WARN` or
 | I382   | argument declared but not used
 | I391   | also a global from src
 | I392   | local variable might cause confusion with a synonymous export from Base
+| I393   | using an existing type as type parameter name is probably a typo
 |        |
 | **I4** | *Usage Info*
 | I472   | assignment in the if-predicate clause

src/Lint.jl

@@ -6,6 +6,7 @@ using Base.Meta
 using Compat
 using Compat.TypeUtils
 using JSON
+import Compat: readline
 
 if isdefined(Base, :unwrap_unionall)
     using Base: unwrap_unionall
@@ -140,7 +141,7 @@ function lintfile(file::AbstractString, code::AbstractString)
 end
 
 function lintstr{T<:AbstractString}(str::T, ctx::LintContext = LintContext(), lineoffset = 0)
-    linecharc = cumsum(map(x->endof(x)+1, @compat(split(str, "\n", keep=true))))
+    linecharc = cumsum(map(x->endof(x)+1, split(str, "\n", keep=true)))
     numlines = length(linecharc)
     i = start(str)
     while !done(str,i)
@@ -242,6 +243,7 @@ function lintexpr(ex::Expr, ctx::LintContext)
     elseif ex.head == :type
         linttype(ex, ctx)
     elseif ex.head == :typealias
+        # TODO: deal with X{T} = Y assignments, also const X = Y
         linttypealias(ex, ctx)
     elseif ex.head == :abstract
         lintabstract(ex, ctx)
@@ -439,9 +441,9 @@ function readandwritethestream(conn,style)
     if style == "original_behaviour"
         # println("Connection accepted")
         # Get file, code length and code
-        file = strip(readline(conn))
+        file = readline(conn)
         # println("file: ", file)
-        code_len = parse(Int, strip(readline(conn)))
+        code_len = parse(Int, readline(conn))
         # println("Code bytes: ", code_len)
         code = Compat.UTF8String(read(conn, code_len))
         # println("Code received")

src/controls.jl

@@ -33,7 +33,7 @@ function lintifexpr(ex::Expr, ctx::LintContext)
         (verconstraint1, verconstraint2) = versionconstraint(ex.args[1])
         if verconstraint1 != nothing
             tmpvtest = ctx.versionreachable
-            ctx.versionreachable = _->(tmpvtest(_) && verconstraint1(_))
+            ctx.versionreachable = x->(tmpvtest(x) && verconstraint1(x))
         end
         insideif(x -> lintexpr(ex.args[2], x), ctx)
         if verconstraint1 != nothing
@@ -42,7 +42,7 @@ function lintifexpr(ex::Expr, ctx::LintContext)
         if length(ex.args) > 2
             if verconstraint2 != nothing
                 tmpvtest = ctx.versionreachable
-                ctx.versionreachable = _->(tmpvtest(_) && verconstraint2(_))
+                ctx.versionreachable = x->(tmpvtest(x) && verconstraint2(x))
             end
             insideif(x -> lintexpr(ex.args[3], x), ctx)
             if verconstraint2 != nothing

src/exprutils.jl

@@ -3,7 +3,8 @@ module ExpressionUtils
 using Base.Meta
 
 export split_comparison, simplify_literal, ispairexpr, isliteral,
-       lexicaltypeof, lexicalfirst, lexicallast, lexicalvalue
+       lexicaltypeof, lexicalfirst, lexicallast, lexicalvalue,
+       withincurly, expand_trivial_calls
 
 # TODO: remove when 0.5 support dropped
 function BROADCAST(f, x::Nullable)
@@ -14,6 +15,24 @@ function BROADCAST(f, x::Nullable)
     end
 end
 
+"""
+    withincurly(ex)
+
+Get just the function part of a function declaration, or just the type head of
+a parameterized type name.
+
+```jldoctest
+julia> using Lint.ExpressionUtils
+
+julia> withincurly(:(Vector{T}))
+:Vector
+
+julia> withincurly(:((::Type{T}){T}))
+:(::Type{T})
+```
+"""
+withincurly(ex) = isexpr(ex, :curly) ? ex.args[1] : ex
+
 """
     split_comparison(::Expr)
 
@@ -117,4 +136,25 @@ alone.
 """
 lexicaltypeof(x) = get(BROADCAST(typeof, lexicalvalue(x)), Any)
 
+"""
+    expand_trivial_calls(x)
+
+Expand the outer layer of trivial calls. Trivial calls are defined as
+expression nodes that almost always lower to calls but are not represented as
+such. The special case lowering of `A*B'` is neglected.
+"""
+function expand_trivial_calls(ex)
+    if isexpr(ex, :(:))
+        Expr(:call, :colon, ex.args...)
+    elseif isexpr(ex, Symbol("'"))
+        Expr(:call, :ctranspose, ex.args...)
+    elseif isexpr(ex, Symbol(".'"))
+        Expr(:call, :transpose, ex.args...)
+    elseif isexpr(ex, :(=>))
+        Expr(:call, :(=>), ex.args...)
+    else
+        ex
+    end
+end
+
 end

src/guesstype.jl

@@ -141,8 +141,11 @@ function guesstype(ex, ctx::LintContext)
         return Any
     end
 
+    ex = ExpressionUtils.expand_trivial_calls(ex)
+
+
     if isexpr(ex, :tuple)
-        ts = Any[]
+        ts = Type[]
         for a in ex.args
             push!(ts, guesstype(a, ctx))
         end
@@ -163,8 +166,11 @@ function guesstype(ex, ctx::LintContext)
     end
 
     if isexpr(ex, :call)
-        # TODO: deal with vararg (...) calls properly
         fn = ex.args[1]
+        if any(x -> isexpr(x, :kw) || isexpr(x, :(...)), ex.args[2:end])
+            # TODO: smarter way to deal with kw/vararg
+            return Any
+        end
         argtypes = map(x -> guesstype(x, ctx), ex.args[2:end])
 
         # check if it's a constructor for user-defined type, and figure
@@ -199,20 +205,9 @@ function guesstype(ex, ctx::LintContext)
 
         # infer return types of Base functions
         obj = stdlibobject(fn)
+        type_argtypes = [isa(t, Type) ? t : Any for t in argtypes]
         if !isnull(obj)
-            inferred = try
-                typejoin(Base.return_types(
-                    get(obj),
-                    Tuple{(isa(t, Type) ? t : Any for t in argtypes)...})...)
-            catch  # error might be thrown if generic function, try using inference
-                if all(typ -> isa(typ, Type) && isleaftype(typ), argtypes)
-                    Core.Inference.return_type(
-                        get(obj),
-                        Tuple{argtypes...})
-                else
-                    Any
-                end
-            end
+            inferred = StaticTypeAnalysis.infertype(get(obj), type_argtypes)
             if inferred ≠ Any
                 return inferred
             end
@@ -232,19 +227,10 @@ function guesstype(ex, ctx::LintContext)
         end
     end
 
-    if isexpr(ex, :(:))
-        return Range
-    end
-
     if isexpr(ex, :curly)
         return Type
     end
 
-    if isexpr(ex, Symbol("'"))
-        fst = guesstype(ex.args[1], ctx)
-        return fst
-    end
-
     if isexpr(ex, :ref) # it could be a ref a[b] or an array Int[1,2,3], Vector{Int}[]
         if isexpr(ex.args[1], :curly) # must be a datatype, right?
             elt = stdlibobject(ex.args[1])
@@ -271,24 +257,11 @@ function guesstype(ex, ctx::LintContext)
         end
         # not symbol, or symbol but it refers to a variable
         partyp = guesstype(ex.args[1], ctx)
-        if typeof(partyp) == Symbol # we are in a context of a constructor of a new type, so it's difficult to figure out the content
+        if isa(partyp, Symbol)
+            # we are in a context of a constructor of a new type, so it's
+            # difficult to figure out the content
             return Any
-        elseif partyp <: UnitRange
-            if length(ex.args) < 2
-                msg(ctx, :E121, ex.args[1], "Lint does not understand the expression")
-                return Any
-            end
-            ktypeactual = guesstype(ex.args[2], ctx)
-            if ktypeactual <: Integer
-                return StaticTypeAnalysis.eltype(partyp)
-            elseif isexpr(ex.args[2], :(:)) # range too
-                return partyp
-            elseif isexpr(ex.args[2], :call) && ex.args[2].args[1] == :Colon
-                return partyp
-            else
-                return Any
-            end
-        elseif partyp <: AbstractArray
+        elseif partyp <: AbstractArray && !(partyp <: Range)
             eletyp = StaticTypeAnalysis.eltype(partyp)
             try
                 nd = ndims(partyp) # This may throw if we couldn't infer the dimension
@@ -315,50 +288,26 @@ function guesstype(ex, ctx::LintContext)
                 end
             end
             return Any
-        elseif partyp <: Associative
-            ktypeexpect = keytype(partyp)
-            vtypeexpect = valuetype(partyp)
-            if length(ex.args) < 2
-                msg(ctx, :E121, ex.args[1], "Lint does not understand the expression")
-                return Any
-            end
-            ktypeactual = guesstype(ex.args[2], ctx)
-            if ktypeactual != Any && !(ktypeactual <: ktypeexpect)
-                msg(ctx, :E518, ex.args[2], "key type expects $(ktypeexpect), " *
-                    "provided $(ktypeactual)")
-            end
-            return vtypeexpect
-        elseif partyp <: AbstractString
-            if length(ex.args) < 2
-                msg(ctx, :E121, ex.args[1], "Lint does not understand the expression")
-                return Any
-            end
-            ktypeactual = guesstype(ex.args[2], ctx)
-            if ktypeactual != Any && !(ktypeactual <: Integer) && !(ktypeactual <: Range)
-                msg(ctx, :E519, ex.args[2], "string[] expects Integer, provided $(ktypeactual)")
-            end
-            if ktypeactual <: Integer
-                return Char
-            end
-            if ktypeactual <: Range
-                return partyp
-            end
-        elseif partyp <: Tuple
-            return StaticTypeAnalysis.eltype(partyp)
-        elseif partyp != Any
-            if ctx.versionreachable(VERSION) && !pragmaexists("$(partyp) is a container type", ctx)
-                msg(ctx, :E521, ex.args[1], "apparent type $(partyp) is not a container type")
+        else
+            argtypes = [guesstype(x, ctx) for x in ex.args]
+            type_argtypes = [isa(t, Type) ? t : Any for t in argtypes]
+            inferred = StaticTypeAnalysis.infertype(getindex, type_argtypes)
+            if ctx.versionreachable(VERSION) && inferred == Union{}
+                indtypes = if length(type_argtypes) == 1
+                    "no indices"
+                else
+                    string("index types ", join(type_argtypes[2:end], ", "))
+                end
+                msg(ctx, :E522, ex,
+                    string("indexing $(type_argtypes[1]) with ",
+                           indtypes,
+                           " is not supported"))
             end
+            return inferred
         end
         return Any
     end
 
-    if isexpr(ex, :(=>))
-        t1 = guesstype(ex.args[1], ctx)
-        t2 = guesstype(ex.args[2], ctx)
-        return Pair{t1,t2}
-    end
-
     if isexpr(ex, :comparison)
         return Bool
     end

src/macros.jl

@@ -52,7 +52,22 @@ end
 istopmacro(ex, mod, mac) = ex in (
     mac,
     GlobalRef(mod, mac),
-    Expr(:(.), Symbol(string(mod), mac)))
+    Expr(:(.), Symbol(string(mod)), mac))
+
+function lintcompat(ex::Expr, ctx::LintContext)
+    if VERSION < v"0.6.0-dev.2746" &&
+       length(ex.args) == 2 && isexpr(ex.args[2], :abstract) &&
+       length(ex.args[2].args) == 1 && isexpr(ex.args[2].args[1], :type)
+        lintexpr(Compat._compat_abstract(ex.args[2].args[1]), ctx)
+    elseif VERSION < v"0.6.0-dev.2746" &&
+           length(ex.args) == 3 && ex.args[2] == :primitive
+        lintexpr(Compat._compat_primitive(ex.args[3]), ctx)
+    elseif length(ex.args) == 2
+        lintexpr(Compat._compat(ex.args[2]), ctx)
+    else
+        msg(ctx, :E437, ex, "@compat called with wrong number of arguments")
+    end
+end
 
 function lintmacrocall(ex::Expr, ctx::LintContext)
     if istopmacro(ex.args[1], Base, Symbol("@deprecate"))
@@ -93,9 +108,9 @@ function lintmacrocall(ex::Expr, ctx::LintContext)
         return
     end
 
-    if ex.args[1] == Symbol("@compat")
-        # TODO: check number of arguments
-        lintexpr(ex.args[2], ctx)
+    if istopmacro(ex.args[1], Compat, Symbol("@compat"))
+        lintcompat(ex, ctx)
+        return
     end
 
     if ex.args[1] == Symbol("@gensym")