Package linting

[ZacLN]

@tkelman, re this I was going to add some new linting checks. Do you want any more rules added?

New Linting rules

• Extended external methods should reference at least one package specific type.
• Check module imports against REQUIRE.
• Check julia REQUIRE version against Travis.
• Check pkg REQUIRE versions against Pkg.available(pkg)
• Check for min REQIURE version of Compat.

[tkelman]

Ooh yeah I've been thinking about how to accomplish exactly this, but am not too familiar with how this/CSTParser/Tokenize/Lint all work. As a (separate?) check, maybe doing the same thing for methods extended from other packages too.

[ZacLN]

Well what I'm going to do today is do a pass over all package/workspace code so that at any step you'll apply some arbitrary lint_xxx(x::EXPR, scope, importedmodules) function to an expression (a CSTParser.EXPR which is pretty much the same as Expr in structure) where scope lists all available symbols both from Base/imported modules and those defined in the current context.

So the above would be something along the lines of pseudocode :

if x.head == :call && x.args[1] in Base && isimported(x.args[1])
    argtypes =  collect(typesof(x.args[2:end]))
    if !any(argstypes in scope)
        return lintmessage("this is bad form")
    end
end

[tkelman]

don't we want :function, or :(=) with lhs a :call, head? guess the other-packages version would just check other imported packages instead of Base

does this work strictly on the text content of a file? does it walk imports or recognize when code is dead (never included or behind some conditional?) or would you run it in catch-all mode on all .jl files and hope no one is including julia files under different extensions that would be missed?

another one for package linting is checking that all directly imported/used packages are listed in REQUIRE, or test/REQUIRE for test-only deps. I get the impression there's already infrastructure here for detecting imports?

[ZacLN]

Yeah that's just my sloppiness on the first point, there's a isfuncdef function floating somewhere in CSTParser.

The LS first builds an tree structure following includes of files in the project so if you lint somefileA the scope is built by first checking which other files include somefileA and starting from there ( #19). There's naive recognition of deadcode sections (i.e. if false) and it traverses expressions maintaining the same scoping rules of evaluation so that an include call inside a function will be followed while traversing the rest of the function body but has no impact on the enclosing scope.

If you include something that doesn't exist you're warned.

[tkelman]

Very cool. Does it attempt to resolve programmatic includes with some heuristic?

[ZacLN]

Not yet, I'm going to add an implementation for joinpath and maybe the simplest of loops

[tkelman]

Hm okay, is there a warning at the moment about possibly-missed includes? Or fall back to "all .jl files" as a next-best guess?

[ZacLN]

No, at the moment it just ignores

[tkelman]

I don't think the code in this package helps me with this one, but while we're writing down things I'd like to lint for - checking that supported Julia versions are all running on Travis.

[ZacLN]

Yeah I don't think that'll fit into general linting but I think it may be worth adding a special tkelman/publish package lint command checking these things. Also: check REQUIRE packages with specified versions against Pkg.available(pkg)?

[oxinabox]

Also maybe check REQUIRE ing Compat.jl, without any min-version?

[ZacLN]

Makes sense

[ZacLN]

@tkelman, available versions for Travis are listed at https://s3.amazonaws.com/julialang right? Which parts am I checking against is it checksums or exe/dmg/tar.gz ?

[tkelman]

we added a new caching layer, I think julialang-s3.julialang.org or something like that. probably unlikely that different platforms' available versions would get out of sync unless I screw something up in uploading

[ZacLN]

Cheers