inaka / elvis Goto Github PK

Basic auth while using the GitHub API results in limited access to some information about private repositories (see here). It is recommended to use OAuth for the type of operations elvis has to perform on repos.

Too deep indentation might signal that a function's logic is too complex. Even if it's not the case, too deeply indented code is hard to read and makes the programmer cramp the code on the far right side of the screen, especially when also using the max-80-chars-per-line rule. This is a real-world example of code cramped on the far right with a really exaggerated case here.

Since this rule is validating that every alphanumeric character between -define( and the first , is upper case, when defining a macro with arguments it fails.

-define(STAGED_CONTENT(Path),
        "git show :" ++ Path).

It should be compatible with Erlang.mk and rebar, as usual

Rule

Don't use if

Rule

Surround operators and commas with spaces.

Options

• List of operators to check/ignore

Implementation

Detect stuff like:

• X++Y
• X,Y
• x ,Y
• etc…

Rule

don't repeat yourself

Implementation

Find exactly the same code repeated in the same module

Rule

every module is referenced in Erlang.mk or rebar.config or Emakefile

Integrate elvis with github

There's a link missing in the README.md.

There's also a way to use elvis as a GitHub [webhook][webhooks] for pull request (PR) events by calling the webhook/1 function. This will add a comment for each rule that is broken by a line in the files associated wiith the PR.

Again with respect to https://github.com/lavrin/dcsp/ after I changed include directives to sidestep #75 Elvis crashes:

> elvis:rock(#{src_dirs => ["src", "include", "test"], rules => []}).
** exception error: no function clause matching erl_parse:abstract(undefined,{5,19}) (erl_parse.yrl, line 935)
     in function  erl_parse:record_fields/1 (erl_parse.yrl, line 792)
     in call from erl_parse:build_typed_attribute/2 (erl_parse.yrl, line 623)
     in call from erl_parse:yeccpars2_483/7 (erl_parse.yrl, line 80)
     in call from erl_parse:yeccpars0/5 (/net/isildur/ldisk/daily_build/17_prebuild_master-opu_o.2014-04-07_20/otp_src_17/bootstrap/lib/parsetools/include/yeccpre.hrl, line 56)
     in call from lists:map/2 (lists.erl, line 1237)
     in call from lists:map/2 (lists.erl, line 1237)
     in call from elvis_code:parse_tree/1 (src/elvis_code.erl, line 68)

Please see this gist for a dbg trace - unfortunately, the trace doesn't tell me much.

Rule

Don't use dynamic function calling outside of behaviour modules with callbacks

Options

• Ignore modules

Rule

Nest your code no more than N levels.

Options

• Max nesting level

Rule

Don't use macros for module or function names

Implementation

check for ?MACROS that translate to an atom and are used in expressions like ?MACRO:something or something:?MACRO

Instead of using the raw url from www.github.com use the API call provided through api.github.com.

Rule

stick to one convention when naming modules (i.e: prefix_module1 vs module2 vs anotherprefix_module3)

Options

• Regex with which to validate module names
• Excluded modules

After setting a webhook GitHub sends a <<"ping">> event to check it works, so elvis should return a 200 response.

Rule

name your state records #state and use -type state():: #state{} in all your OTP modules.

Rule

Indent using {N spaces|tabs}

Options

• Indentation character
• (if using spaces) number of spaces

Implementation

Check in each line, how much more indented it is against the previous one.

The position argument provided when creating a PR review comment has to be relative to the file's diff and not to the file itself.

A missing space inside a string (i.e. "^ *[-]define *[(]([^,]+)") is reported as a rule failure.

With the following repo layout:

.
├── Makefile
├── README.md
├── include
│   └── dcsp.hrl
├── src
│   ├── dcsp.app.src
│   ├── dcsp.erl
│   ├── dcsp_agent.erl
│   ├── dcsp_problem.erl
│   ├── dcsp_problem_logic.erl
│   ├── dcsp_solver.erl
│   ├── dcsp_sup.erl
│   ├── ex1.erl
│   └── nqueens.erl
└── test
    └── dcsp_SUITE.erl

and dcsp.erl (and others) including dcsp.hrl like this (note no leading include/):

-include("dcsp.hrl").

Elvis (commit 647fd5a) can't find dcsp.hrl:

> elvis:rock(#{src_dirs => ["src", "include", "test"], rules => []}).
** exception error: no match of right hand side value {error,{not_found,"dcsp.hrl"}}
     in function  elvis_code:parse_tree/1 (src/elvis_code.erl, line 65)
     in call from elvis_utils:parse_tree/1 (src/elvis_utils.erl, line 56)
     in call from elvis_utils:load_file_data/1 (src/elvis_utils.erl, line 68)
     in call from lists:map/2 (lists.erl, line 1237)
     in call from elvis:rock/1 (src/elvis.erl, line 52)

The repo in question is https://github.com/lavrin/dcsp/.

Rule

Stick to N chars per line

Options

• Max line length

The code sample that uses elvis from the shell by providing the configuration as an argument uses a proplists, which is outdated since the configuration is now specified as a map.

Rule

Use iolists instead of string concatenation whenever possible

If there is an elvis.config file in the repository's root folder (on the last commit of the pull request or in master), then use this as the configuration for running elvis on project's file.

Rule

Don't design your system using god modules

Options

• Max number of exported functions
• Max number of expressions in the generated code
• Max number of lines of code

Implementation

The number of exported functions and expressions in generated code can be obtained using beam_lib-related functions
The number of lines of code, maybe too

Rule

Macros should be named in ALL_UPPER_CASE

Options

No options

Get the source code for a file and process the parse tree only once before applying rules.

Rule

use -callback instead of behavior_info/1 for your behavior definitions

Options

• None

Rule

Don't write spaghetti code

Create an escript that basically runs elvis:rock(…). Use the tool for CLI getopt.

If there is no elvis.config file in the repo's master branch, the webhook fails to process the event.

Rule

in your rebar.config or Erlang.mk, specify a tag or commit, but not master

Rule

Write functions with executable paths no longer than N.

Options

• N: max length of executable paths.
• Ignored modules

Rule

Try to always separate NON-EXPORTED and EXPORTED functions in groups, with the exported ones first, unless it helps readability and code discovery.

My name's Radosław, but the site displays it as RadosÅ‚aw. There's no mention of "encoding" or "charset" in the page source, so the browser-default encoding is used, and that's ISO-8859-2 in my case (I'm from Poland), but the site is in fact in UTF-8. I guess <meta charset='utf-8'> in the head section would suffice.

Let elvis run on PRs and/or commits

• from Erlang console
• from CLI

Error message:

escript: exception error: no match of right hand side value
                 {error,{{49,36},
                         aleppo_parser,
                         ["syntax error before: ","..."]}}
  in function  elvis_code:parse_tree/2 (src/elvis_code.erl, line 67)
  in call from elvis_utils:parse_tree/2 (src/elvis_utils.erl, line 57)
  in call from elvis_utils:load_file_data/2 (src/elvis_utils.erl, line 69)
  in call from lists:map/2 (lists.erl, line 1237)
  in call from lists:map/2 (lists.erl, line 1237)
  in call from elvis:rock/1 (src/elvis.erl, line 53)
  in call from elvis:process_commands/2 (src/elvis.erl, line 152)
  in call from elvis:process_options/2 (src/elvis.erl, line 131)

Create a wiki page listing the rules with their options and samples.

How about making the output compatible with the CheckStyle file format? This will allow to run the tool from jenkins and publish the results of the build without any translations

For big projects (e.g. erlang/otp, ejabberd), the initial processing of files takes too long and the user is left waiting without any feedback. It would be nice to display information related to what file is being processed before showing the actual output.

Rule

don't use your ignored variables (_Var)

Options

• None

Rule

avoid using records in your specs, use types.

…and possibly add a hook to run it.

• Man Page

Using github-pages, build a simple site like HoundCI.com

Make the system run with

elvis:rock(…)

or something similar (i.e. let it be ready to run from an Erlang console), following these rules:

• The code to be analysed should be src/*.erl or given as a param
• The list of rules to validate should be configurable by app.config and only one or two very simple rules (e.g. 80 chars a line) should be implemented
• The results should be in the simplest possible form
• Add Tests
• Write Documentation