Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New linter #299

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ branch](https://github.com/leafo/moonscript/tree/binaries)

## License (MIT)

Copyright (C) 2015 by Leaf Corcoran
Copyright (C) 2015 by Leaf Corcoran, Nils Nordman

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand All @@ -64,4 +64,4 @@ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
THE SOFTWARE.
12 changes: 6 additions & 6 deletions bin/moon.moon
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ errors = require "moonscript.errors"

unpack = util.unpack

opts, ind = alt_getopt.get_opts arg, "cvhd", {
opts, ind = alt_getopt.get_opts _G.arg, "cvhd", {
version: "v"
help: "h"
}
Expand All @@ -26,7 +26,7 @@ print_err = (...) ->
io.stderr\write msg .. "\n"

print_help = (err) ->
help = help\format arg[0]
help = help\format _G.arg[0]

if err
print_err err
Expand All @@ -44,15 +44,15 @@ run = ->
require("moonscript.version").print_version!
os.exit!

script_fname = arg[ind]
script_fname = _G.arg[ind]

unless script_fname
print_help "repl not yet supported"

new_arg = {
[-1]: arg[0],
[0]: arg[ind],
select ind + 1, unpack arg
[-1]: _G.arg[0],
[0]: _G.arg[ind],
select ind + 1, unpack _G.arg
}

local moonscript_chunk, lua_parse_error
Expand Down
19 changes: 13 additions & 6 deletions bin/moonc
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,14 @@ if opts.w then
-- build function to check for lint or compile in watch
local handle_file
if opts.l then
local lint = require "moonscript.cmd.lint"
handle_file = lint.lint_file
local lint = require "moonscript.lint"
handle_file = function(file)
local res, err = lint.lint_file(file)
if res and #res > 0 then
return file .. "\n\n" .. lint.format_inspections(res)
end
return nil, err
end
else
handle_file = compile_and_write
end
Expand Down Expand Up @@ -220,16 +226,17 @@ if opts.w then
io.stderr:write("\nQuitting...\n")
elseif opts.l then
local has_linted_with_error;
local lint = require "moonscript.cmd.lint"
local lint = require "moonscript.lint"
for _, tuple in pairs(files) do
local fname = tuple[1]
local res, err = lint.lint_file(fname)
if res then
if res and #res > 0 then
has_linted_with_error = true
io.stderr:write(res .. "\n\n")
local inspections = lint.format_inspections(res)
io.stderr:write(fname .. "\n\n" .. inspections .. "\n\n")
elseif err then
has_linted_with_error = true
io.stderr:write(fname .. "\n" .. err.. "\n\n")
io.stderr:write(fname .. "\n\n" .. err.. "\n\n")
end
end
if has_linted_with_error then
Expand Down
10 changes: 5 additions & 5 deletions bin/splat.moon
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@
lfs = require "lfs"
alt_getopt = require "alt_getopt"

import insert, concat from table
import dump, split from require "moonscript.util"
import insert from table
import split from require "moonscript.util"

opts, ind = alt_getopt.get_opts arg, "l:", {
opts, ind = alt_getopt.get_opts _G.arg, "l:", {
load: "l"
}

if not arg[ind]
if not _G.arg[ind]
print "usage: splat [-l module_names] directory [directories...]"
os.exit!

dirs = [a for a in *arg[ind,]]
dirs = [a for a in *_G.arg[ind,]]

normalize = (path) ->
path\match("(.-)/*$").."/"
Expand Down
161 changes: 141 additions & 20 deletions docs/command_line.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,16 @@ A full list of flags can be seen by passing the `-h` or `--help` flag.
### Linter

`moonc` contains a [lint][1] tool for statically detecting potential problems
with code. The linter has two tests: detects accessed global variables,
detect unused declared variables. If the linter detects any issues with a file,
the program will exit with a status of `1`.
with code. The linter can detect the following classes of potential errors:

- global variable accesses
- declared but unused variables
- unused parameter declarations
- unused loop variables
- declaration shadowing

If the linter detects any issues with a file, the program will exit with a
status of `1`.

You can execute the linter with the `-l` flag. When the linting flag is
provided only linting takes place and no compiled code is generated.
Expand All @@ -149,7 +156,46 @@ moonc -l file1.moon file2.moon
Like when compiling, you can also pass a directory as a command line argument
to recursively process all the `.moon` files.

#### Global Variable Checking
#### Linter configuration file

When linting a file, the linter will look for a file named either
`lint_config.moon` or `lint_config.lua` in the same directory as the file, or in
one of the parent directories. Written in Moonscript or Lua respectively, this
file can provide additional configuration for the linter. The specific
configuration options available are discussed in the sections below, but
generally speaking, the linter configuration consists of either boolean flags or
whitelisting lists. In the case of the lists, they are defined in a way that
lets you easily define different lists for different files or directories. As an
example, consider the following whitelist for globals:

```moononly
-- lint_config.moon
{
whitelist_globals: {
['.']: { 'foo' }

['sub_dir/']: { 'bar' }

['sub_dir/example.moon']: {
'zed',
'[A-Z]%w+'
}
}
}
```

The structure of a list is that it's a table with keys that are Lua patterns
which are matched against the path of the file being linted, with _all_ of the
matching entries being considered for the file. The entries themselves are
typically plain strings, but can also be Lua patterns. The above list would thus
result in `foo` being whitelisted for all files in the project, while files
below the `sub_dir` directory would have both `foo` and `bar` whitelisted.
Finally, for the specific file "sub_dir/example.moon" all four entries would be
used for whitelisting - `foo`, `bar`, `zed` and `[A-Z]%w+`. The latter, being a
pattern would whitelist all occurrences matching the pattern, such as "Foo",
"Bar2" and "FooBar2".

#### Global Variable Accesses

It's considered good practice to avoid using global variables and create local
variables for all the values referenced. A good case for not using global
Expand Down Expand Up @@ -192,7 +238,7 @@ Outputs:
==================================
> my_nmuber + 10

#### Global Variable Whitelist
##### Global Variable Whitelist

In most circumstances it's impossible to avoid using some global variables. For
example, to access any of the built in modules or functions you typically
Expand All @@ -208,11 +254,8 @@ global functions (like `describe`, `before_each`, `setup`) to make writing
tests easy.

It would be nice if we could allow all of those global functions to be called
for `.moon` files located in the `spec/` directory. We can do that by creating
a `lint_config` file.

`lint_config` is a regular MoonScript or Lua file that provides configuration
for the linter. One of those settings is `whitelist_globals`.
for `.moon` files located in the `spec/` directory. We can do that by providing
a `whitelist_globals` list in the `lint_config` file.

To create a configuration for Busted we might do something like this:

Expand Down Expand Up @@ -242,23 +285,16 @@ $ moonc -l .

The whitelisted global references in `spec/` will no longer raise notices.

The `whitelist_globals` property of the `lint_config` is a table where the keys
are Lua patterns that match file names, and the values are an array of globals
that are allowed.

Multiple patterns in `whitelist_globals` can match a single file, the union of
the allowed globals will be used when linting that file.

#### Unused Variable Assigns

Sometimes when debugging, refactoring, or just developing, you might leave
behind stray assignments that aren't actually necessary for the execution of
your code. It's good practice to clean them up to avoid any potential confusion
they might cause.

The unused assignment detector keeps track of any variables that are assigned,
and if they aren't accessed in within their available scope, they are reported
as an error.
The unused assignment detector keeps track of any variables that are assigned or
otherwise declared, and if they aren't accessed in within their available scope,
they are reported as an error.

Given the following code:

Expand Down Expand Up @@ -288,5 +324,90 @@ _, name, _, count = unpack item
print name, count
```

There are very few cases where one would need additional whitelisting for unused
variables, but it's possible that there are some, e.g. in tests. For this
purpose additional whitelisting can be specified using the `whitelist_unused`
configuration list:

```moononly
-- lint_config.moon
{
whitelist_unused: {
['spec/']: {
'my_unused'
}
}
}
```

#### Unused parameter declarations

The linter can also detect and complain about declared but unused function
parameters. This is not enabled by default, as it's very common to have unused
parameters. E.g. a function might follow an external API and still wants to
indicate the available parameters even though not all are used.

To enable this detection, set the `report_params` configuration option to
`true`:

```moononly
-- lint_config.moon
{
report_params: true
}
```

The linter ships with a default configuration that whitelists any parameter
starting with a '_', providing a way of keeping the documentational aspects for
a function and still pleasing the linter. Other whitelisting can be specified by
adding a `whitelist_params' list to the linter configuration (please note that
the default whitelisting is not used when the configuration specifies a list).

#### Unused loop variables

Unused loop variables are detected. It's possible to disable this completely in
the configuration by setting the `report_loop_variables` variable to `false`, or
to provide an explicit whitelist only for loop variables. The linter ships with
a default configuration that whitelists the arguments 'i' and 'j', or any
variable starting with a '_'.

Other whitelisting can be specified by adding a `whitelist_loop_variables' list
to the linter configuration (please note that the default whitelisting is not
used when the configuration specifies a list).

#### Declaration shadowing

Declaration shadowing occurs whenever a declaration shadows an earlier
declaration with the same name. Consider the following code:

```moononly
my_mod = require 'my_mod'

-- [.. more code in between.. ]

for my_mod in get_modules('foo')
my_mod.bar!
```

While it in the example above is rather clear that the `my_mod` declared in the
loop is different from the top level `my_mod`, this can quickly become less
clear should more code be inserted between the for declaration and later usage.
At that point the code becomes ambiguous. Declaration shadowing helps with this
by ensuring that each variable is defined at most once, in an unambiguous
manner:

```bash
$ moonc -l lint_example.moon
```

line 5: shadowing outer variable - `my_mod`
===========================================
> for my_mod in get_modules('foo')


The detection can be turned off completely by setting the `report_shadowing`
configuration variable to false, and the whitelisting can be configured by
specifying a `whitelist_shadowing` configuration list.

[1]: http://en.wikipedia.org/wiki/Lint_(software)

28 changes: 28 additions & 0 deletions lint_config.moon
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
whitelist_globals: {
'.': {
},

'parse.moon': {
'[A-Z][a-z]+'
},

spec: {
'after_each',
'async',
'before_each',
'describe',
'it',
'settimeout',
'setup',
'spy',
'teardown',

'hello'
}
}

whitelist_unused: {
'spec/import_spec': { 'hello', 'foo' }
}
}
3 changes: 2 additions & 1 deletion moonscript-dev-1.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ build = {
["moonscript"] = "moonscript/init.lua",
["moonscript.base"] = "moonscript/base.lua",
["moonscript.cmd.coverage"] = "moonscript/cmd/coverage.lua",
["moonscript.cmd.lint"] = "moonscript/cmd/lint.lua",
["moonscript.cmd.moonc"] = "moonscript/cmd/moonc.lua",
["moonscript.cmd.watchers"] = "moonscript/cmd/watchers.lua",
["moonscript.compile"] = "moonscript/compile.lua",
Expand All @@ -38,6 +37,8 @@ build = {
["moonscript.dump"] = "moonscript/dump.lua",
["moonscript.errors"] = "moonscript/errors.lua",
["moonscript.line_tables"] = "moonscript/line_tables.lua",
["moonscript.lint"] = "moonscript/lint/init.lua",
["moonscript.lint.config"] = "moonscript/lint/config.lua",
["moonscript.parse"] = "moonscript/parse.lua",
["moonscript.parse.env"] = "moonscript/parse/env.lua",
["moonscript.parse.literals"] = "moonscript/parse/literals.lua",
Expand Down
2 changes: 1 addition & 1 deletion moonscript/base.moon
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ compile = require "moonscript.compile"
parse = require "moonscript.parse"

import concat, insert, remove from table
import split, dump, get_options, unpack from require "moonscript.util"
import split, get_options, unpack from require "moonscript.util"

lua = :loadstring, :load

Expand Down
Loading