Skip to content

Commit

Permalink
Fix #252 ~ Update code to julia 1.0 (#256)
Browse files Browse the repository at this point in the history
* Try to fix precompile issues for Julia 1.0

* Try to fix some static analysis test cases

* Empty `Tuple` method dispatch

See documentation on `Vararg`

* Fix handling of `Vector`

`Vector` has no types ⇒ `length(T.types) == 0`

* Remove `try` without `catch`

Why was it there to begin with?

* basic.jl runs ok

Added a heuristic on `lintpkg` to distinguish a package path from a package name.
Compat `renamed is_windows`.

* Re-write and documentation of `_lintstr`

It was *REALLY* hard to understand the intent of the function.
The code now is still verbose, but each _code block_ has its specific
purpose. Before this we had several stateful variables and conditions
that may or may not be triggered

* Adjust versions

No going back

* Using Pkg

* `isnull(…)` → `… == nothing`

* Remove more `isnull`s and helper function `BROADCAST`

* `contains` → `occursin`

* Minor code update

* `isbits` applies to objects, `isbitstype` to types

`t` was supposed to be a type, not an object

* Break expression from-string producer code

* Use `let …` statements for several reasons

● It's visually easy to distinguish between two different test cases
● Ensures that global state will not change between tests

* Colon detection in expression is bad.

At least now I know how to fix it

* Correctly detect a `:`

* Break `guesstype` into multi-dispatch code.

This will make testing much easier.

Inspired on
https://pixorblog.wordpress.com/2018/02/23/julia-dispatch-enum-vs-type-comparison/ (or
https://www.juliabloggers.com/julia-dispatching-enum-versus-type/ )

* Fix `guesstype` tests

* Remove more `get(…)` calls from version<07's nullables

* `contains` → `occursin`

* `@lintpragma` seems to be parsed differently now

* sed -i s/contains/occursin/g *.jl

* forgot that `occursin` has needle as 1st argument 😒

* `--compilecache=no` → `--compiled-modules=no`

Per #256 (comment)

* Remove `Test` as it belongs to std lib

* Simple updates around `get(…)` and `isnull(…)`

* Remove `occursin(…)` as they only clog testing now

They were meant to test the error being popped, but this was before we
had codes for errors.

* Base.var → Statistics.var in v1.0

Also, cosmetics

* Using another `Base.+` instead of `Base.var`

Friendly reminder that Base.var → Statistics.var in v1.0

* Remove `occursin`s

* Reformat dict tests

* Looks like this test is no longer broken

* `Void` → `Cvoid`

See JuliaLang/julia#25082 for more context

* Remove `occursin` (legacy testing before using codes)

* Remove `occursin`

* Forward `isknownerror` arguments from `infertype`

* Chasing `shadowed variable` problem

This error is the root of many tests failing

* Drop versions prior to v1.0

* `… ≠ nothing` → `… !== nothing`

#256 (comment)

* Fix parsing

Lines were being parsed _regardless_ of whether they were parsed
before.

Now the expression _and_ expression offset is being reported to keep
track and omit lines already parsed

* Mark TODO

* Line iterator

* Must handle (SubString) indices and not Strings

We have to track offsets, so we need indices to the original
string. It surprises me that `split(…)` does not return _or_ handle an
iterator (forced Array output)

* Rewrite test

* Offset lines and most of expression iterator

Re-wrote `each_line_iterator` so we can actually use it as
iterator (with `iterate(…)`)

* Inline functions, use offsets as lines

* Iterators are hard

* Redid the API to have line's offsets

The code is made to report expression and line offsets, but I wrapped
around those iterators to provide a "just expression" API

* Try to replace previous parsing (doesn't work)

* Missing try-catch

Do mind that pre-compile forced `lintstr` seems to be reporting errors

* delete legacy file
  • Loading branch information
FelipeLema authored and TotalVerb committed Dec 21, 2018
1 parent 40fa351 commit f9cdca7
Show file tree
Hide file tree
Showing 67 changed files with 958 additions and 793 deletions.
5 changes: 2 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ os:
- linux
- osx
julia:
- 0.5
- 0.6
- 1.0
- nightly
notifications:
email: false
after_success:
- julia -e 'cd(Pkg.dir("Lint")); Pkg.add("Coverage"); using Coverage; Coveralls.submit(process_folder())';
script:
- if [[ -a .git/shallow ]]; then git fetch --unshallow; fi
- julia --compilecache=no -e 'Pkg.clone(pwd()); Pkg.build("Lint"); Pkg.test("Lint"; coverage=true)';
- julia --compiled-modules=no -e 'using Pkg; Pkg.clone(pwd()); Pkg.build("Lint"); Pkg.test("Lint"; coverage=true)'
- julia -e 'using Lint' # loading after precompilation
4 changes: 1 addition & 3 deletions src/Lint.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ using Compat
using Compat: TypeUtils, readline
using JSON
using AutoHashEquals
using Printf

if isdefined(Base, :unwrap_unionall)
using Base: unwrap_unionall
Expand All @@ -25,9 +26,6 @@ const SIMILARITY_THRESHOLD = 10.0
macro lintpragma(s)
end

# needed for BROADCAST
include("compat.jl")
using .LintCompat
include("exprutils.jl")
using .ExpressionUtils

Expand Down
42 changes: 23 additions & 19 deletions src/abstracteval.jl
Original file line number Diff line number Diff line change
@@ -1,59 +1,63 @@
"""
abstract_eval(ctx::LintContext, ex) :: Nullable
abstract_eval(ctx::LintContext, ex) :: Union{Any, Nothing}
Like `eval`, but does it in the current context and without any dynamism.
Returns `Nullable()` if the result can't be evaluated.
Returns `nothing` if the result can't be evaluated.
"""
abstract_eval(ctx::LintContext, ex::Symbol) =
flatten(BROADCAST(extractobject, lookup(ctx, ex)))
function abstract_eval(ctx::LintContext, ex::Symbol)
let lu = lookup(ctx, ex)
if lu !== nothing
extractobject(lu)
end
end
end

"""
abstract_eval(ctx::LintContext, ex::Expr) :: Nullable
abstract_eval(ctx::LintContext, ex::Expr) :: Union{Any, Nothing}
If the given expression is curly, and each component of the curly is a constant
object in the given `ctx`, construct the object `x` as would have been done in
the program itself, and return `Nullable(x)`.
the program itself, and return `x`.
Otherwise, if the given expression is `foo.bar`, and `foo` is a standard
library object with attribute `bar`, then construct `foo.bar` as would be done
in the program itself and return it.
Otherwise, return `Nullable()`.
Otherwise, return `nothing`.
"""
abstract_eval(ctx::LintContext, ex::Expr) = begin
if isexpr(ex, :curly)
# TODO: when 0.5 support dropped, remove [...] around ctx
objs = abstract_eval.([ctx], ex.args)
if all(!isnull, objs)
objs = [abstract_eval(ctx, arg) for arg in ex.args]
if all(e->e!==nothing, objs)
try
Nullable(Core.apply_type(get.(objs)...))
Core.apply_type(objs...)
catch
Nullable()
nothing
end
else
Nullable()
nothing
end
elseif isexpr(ex, :(.))
head = ex.args[1]
tail = ex.args[2].value
obj = abstract_eval(ctx, head)
if !isnull(obj)
if obj !== nothing
try
Nullable(getfield(get(obj), tail))
getfield(obj, tail)
catch
Nullable()
nothing
end
else
Nullable()
nothing
end
else
Nullable()
nothing
end
end

"""
abstract_eval(ctx::LintContext, ex)
Return the literal embedded within a `Nullable{Any}`.
Return the literal embedded within a `Union{Any, Nothing}`.
"""
abstract_eval(ctx::LintContext, ex) = lexicalvalue(ex)
6 changes: 3 additions & 3 deletions src/ast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ function lintexpr(ex::Expr, ctx::LintContext)
lintifexpr(ex, ctx)
elseif ex.head == :(=) && typeof(ex.args[1])==Expr && ex.args[1].head == :call
lintfunction(ex, ctx)
elseif !isnull(expand_assignment(ex))
ea = get(expand_assignment(ex))
elseif expand_assignment(ex) !== nothing
ea = expand_assignment(ex)
lintassignment(Expr(:(=), ea[1], ea[2]), ctx)
elseif ex.head == :local
lintlocal(ex, ctx)
Expand Down Expand Up @@ -99,7 +99,7 @@ function lintexpr(ex::Expr, ctx::LintContext)
lintmacrocall(ex, ctx)
elseif ex.head == :call
lintfunctioncall(ex, ctx)
elseif ex.head == :(:)
elseif ex.head == :(:) # TODO(felipe) check for `Colon()`
lintrange(ex, ctx)
elseif ex.head == :(::) # type assert/convert
lintexpr(ex.args[1], ctx)
Expand Down
2 changes: 1 addition & 1 deletion src/blocks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ function expr_similar_score(e1::Expr, e2::Expr, base::Float64 = 1.0)
return score
end

function test_similarity_string{T<:AbstractString}(str::T)
function test_similarity_string(str::T) where T<:AbstractString
i = start(str)
firstexpr = nothing
lastexpr = nothing
Expand Down
58 changes: 29 additions & 29 deletions src/cli.jl
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
include("expression_iterator.jl")
using Compat
function lintpkg(pkg::AbstractString)
p = joinpath(Pkg.dir(pkg), "src", basename(pkg) * ".jl")
if !ispath(p)
throw("cannot find path: " * p)
if occursin("/", pkg) # pkg is a file path
return LintResult(lintpkgforfile(pkg))
end

try
p = Base.find_package(pkg)
LintResult(lintpkgforfile(p))
catch
throw("cannot find package: " * pkg)
end
LintResult(lintpkgforfile(p))
end

"""
Expand All @@ -15,7 +22,7 @@ If file is in base lint all files in base dir.
function lintpkgforfile(path::AbstractString, ctx::LintContext=LintContext())
path = abspath(path)
if ispath(ctx.path)
if is_windows()
if Sys.iswindows()
len = count(x -> x == '\\', path)
else
len = count(x -> x == '/', path) - 1
Expand Down Expand Up @@ -74,34 +81,27 @@ function lintfile(f::AbstractString, code::AbstractString)
LintResult(msgs)
end

"Lint over each expression in each line.
Calls `lintexpr` over each parseable-parsed expression.
Each parse is called over each line."
function _lintstr(str::AbstractString, ctx::LintContext, lineoffset = 0)
linecharc = cumsum(map(x->endof(x)+1, split(str, "\n", keep=true)))
numlines = length(linecharc)
i = start(str)
while !done(str,i)
problem = false
ex = nothing
linerange = searchsorted(linecharc, i)
if linerange.start > numlines # why is it not donw?
break
else
linebreakloc = linecharc[linerange.start]
non_empty_lines=split(str, "\n", limit=0, keepempty=false)
offset_where_last_expression_ends=nothing
try
for (ex, line_begin, line_end) in ExpressionIterator.each_expression_and_offset(str)
# inform context of current line
ctx.line = ctx.lineabs = line_end + lineoffset +1
lintexpr(ex, ctx)
end
if linebreakloc == i || isempty(strip(str[i:(linebreakloc-1)]))# empty line
i = linebreakloc + 1
continue
catch y
# report an unexpected error
# end-of-input and parsing errors are expected
if typeof(y) != Meta.ParseError || y.msg != "end of input"
msg(ctx, :E111, string(y))
end
ctx.line = ctx.lineabs = linerange.start + lineoffset
try
(ex, i) = parse(str,i)
catch y
if typeof(y) != ParseError || y.msg != "end of input"
msg(ctx, :E111, string(y))
end
break
end
lintexpr(ex, ctx)
end

end

"""
Expand Down
41 changes: 0 additions & 41 deletions src/compat.jl

This file was deleted.

5 changes: 3 additions & 2 deletions src/curly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const CURLY_CONTRACTS = Dict{Symbol, Any}(

function lintcurly(ex::Expr, ctx::LintContext)
head = ex.args[1]
if head == :Ptr && length(ex.args) == 2 && ex.args[2] == :Void
if head == :Ptr && length(ex.args) == 2 && ex.args[2] == :Nothing
return
end
contract = get(CURLY_CONTRACTS, head, nothing)
Expand All @@ -28,8 +28,9 @@ function lintcurly(ex::Expr, ctx::LintContext)
continue # grandfathered
else
t = guesstype(a, ctx)
if !(t <: Type || t == Symbol || isbits(t) || t == Any)
if !(t <: Type || t == Symbol || isbitstype(t) || t == Any)
msg(ctx, :W441, a, "probably illegal use inside curly")

elseif contract != nothing
if i - 1 > length(contract)
msg(ctx, :W446, head, "too many type parameters")
Expand Down
4 changes: 2 additions & 2 deletions src/dict.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function lintdict(ex::Expr, ctx::LintContext)
if ispairexpr(a)
keyexpr = lexicalfirst(a)
lit = lexicalvalue(keyexpr)
if !isnull(lit)
if lit !== nothing
if keyexpr in ks
msg(ctx, :E334, keyexpr, "duplicate key in Dict")
end
Expand All @@ -16,7 +16,7 @@ function lintdict(ex::Expr, ctx::LintContext)
for (j,s) in [(lexicalfirst,ktypes), (lexicallast,vtypes)]
kvexpr = j(a)
typeguess = lexicaltypeof(kvexpr)
if isleaftype(typeguess)
if isconcretetype(typeguess)
push!(s, typeguess)
elseif isexpr(kvexpr, :call) && in(kvexpr.args[1], [:Date, :DateTime])
# TODO: use the existing guesstype infrastructure
Expand Down
6 changes: 3 additions & 3 deletions src/dynamic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
Dynamically import the top-level module given by `sym`, and return it if
possible.
"""
function dynamic_import_toplevel_module(sym)::Nullable{Module}
function dynamic_import_toplevel_module(sym)::Union{Module, Nothing}
info("dynamic import: $sym")
try
eval(Main, :(import $sym))
Nullable(getfield(Main, sym))
getfield(Main, sym)
catch
Nullable()
nothing
end
end
Loading

0 comments on commit f9cdca7

Please sign in to comment.