jfmengels/elm-review
1
1
Fork 0
mirror of https://github.com/jfmengels/elm-review.git synced 2024-11-26 07:44:22 +03:00
Code Issues Packages Projects Releases Wiki Activity

Rename project to elm-review

Browse Source
This commit is contained in:
Jeroen Engels 2019-09-28 17:06:15 +02:00
parent 2b37947669
commit cf344cb904
11 changed files with 71 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
CHANGELOG.md
View File

@ -1,57 +1,5 @@
# Changelog
## 3.0.0 -> 4.0.0
## 1.0.0
v4.0.0 is a full rewrite of the project, with plenty of API, design, and opinion
changes. It is best to consider this version as a new project rather than a
continuation of the previous version. If you were using previous versions of
this project, it would be best to re-read the documentation from scratch.
In case you are wondering, here are the API changes:
```
---- ADDED MODULES - MINOR ----
Lint.Fix
Lint.Project
Lint.Rule
Lint.Test
---- REMOVED MODULES - MAJOR ----
Lint.Rules.NoDebug
Lint.Types
---- Lint - MAJOR ----
Added:
type Error
errorDetails : Lint.Error -> List.List String.String
errorFixes : Lint.Error -> Maybe.Maybe (List.List Lint.Fix.Fix)
errorMessage : Lint.Error -> String.String
errorModuleName : Lint.Error -> Maybe.Maybe String.String
errorRange : Lint.Error -> Elm.Syntax.Range.Range
errorRuleName : Lint.Error -> String.String
Removed:
doNothing : context -> Direction (Node a) -> ( List LintError, context )
lintSource :
List ( Severity, LintRule )
-> String
-> Result (List String) (List ( Severity, LintError ))
parseSource : String -> Result (List String) File
visitExpression :
LintRuleImplementation context
-> Node Expression
-> ( List LintError, context )
Changed:
- lint : File -> LintRuleImplementation context -> List LintError
+ lint :
List.List Lint.Rule.Rule
-> Lint.Project.Project
-> { path : String.String, source : String.String }
-> List.List Lint.Error
```
This is the first release of `elm-review`. The project was previously named [`elm-lint`](https://github.com/jfmengels/elm-lint/).

65
README.md
View File

@ -1,33 +1,42 @@
# elm-lint
# elm-review
![](https://travis-ci.com/jfmengels/elm-lint.svg?branch=master)
![](https://travis-ci.com/jfmengels/elm-review.svg?branch=master)
`elm-lint` lints [Elm](https://elm-lang.org/) source code, to add additional guarantees to your project.
`elm-review` analyzes [Elm](https://elm-lang.org/) source code, to add additional guarantees to your project.
![elm-lint reporter output](https://github.com/jfmengels/elm-lint/blob/master/documentation/images/elm-lint-report.png?raw=true)
![elm-review reporter output](https://github.com/jfmengels/elm-review/blob/master/documentation/images/elm-review-report.png?raw=true)
## What does `elm-lint` do?
## What does `elm-review` do?
`elm-lint` analyzes your [Elm](https://elm-lang.org/) source code, and tries to recognize patterns that may be considered harmful or can be improved upon.
`elm-review` analyzes your [Elm](https://elm-lang.org/) source code, and tries to recognize patterns that may be considered harmful or can be improved upon.
If you are familiar with [ESLint](http://eslint.org/) from JavaScript, this is pretty much the same idea.
The idea is to improve your Elm source code base, after it passes compilation and [elm-format](https://github.com/avh4/elm-format) has been run on it.
You can configure your project to be analyzed by different "rules". You can find [some in the Elm packages](https://klaftertief.github.io/elm-search/?q=Lint.Rule.Rule), or you can write your own rules to enforce rules specific to your project or team. A few use-cases:
You can configure your project to be analyzed by different "rules". You can find [some in the Elm packages](https://klaftertief.github.io/elm-search/?q=Review.Rule.Rule), or you can write your own rules to enforce rules specific to your project or team. A few use-cases:
- You noticed a bad pattern in your codebase, wrote a nice module to handle the pattern better, and want to prevent your team from writing that pattern from now on. You can then write a rule to detect that pattern and have it suggest using your module instead. If you don't, you need to communicate this well to all your teammates, but there is no way to prevent the bad pattern from occurring again.
- You often notice that strings in your codebase contain very common typos, or bad use of punctuation (like a missing space after `;`).
- You have one module in your codebase which centralizes some data used across the application (the paths to all the images, a list of all the available colors, ...), but you keep finding new definitions of that data across the codebase.
- You published a library in the Elm package registry, and notice some pitfalls that users can fall in, that all your research for a better API does not prevent. You can then publish a separate package with rules preventing those pitfalls, should the user use `elm-lint` in their project.
- You published a library in the Elm package registry, and notice some pitfalls that users can fall in, that all your research for a better API does not prevent. You can then publish a separate package with rules preventing those pitfalls, should the user use `elm-review` in their project.
When solving a problem, a good API is a usually a better solution than writing a linter rule. But in some cases, even if you've written a good API, nothing prevents teammates or yourself from falling in the same unwanted patterns as before, especially when dealing with primitive values or constructs.
When solving a problem, a good API is a usually a better solution than writing a review rule. But in some cases, even if you've written a good API, nothing prevents teammates or yourself from falling in the same unwanted patterns as before, especially when dealing with primitive values or constructs.
When introducing `elm-lint` or new rules to your project and team, you should discuss it with them first. It is easy to think that some patterns are always better and want to enforce them, where in reality some edge cases exist where they aren't wanted. Also, people don't usually like it when seemingly arbitrary rules are imposed on them, especially if it relates to code style, so be sure to talk with them and explain the rationale.
When introducing `elm-review` or new rules to your project and team, you should discuss it with them first. It is easy to think that some patterns are always better and want to enforce them, where in reality some edge cases exist where they aren't wanted. Also, people don't usually like it when seemingly arbitrary rules are imposed on them, especially if it relates to code style, so be sure to talk with them and explain the rationale.
## Try it
The preferred method, if you have `Node.js` and `npm` installed, is to use [`node-elm-lint`](https://github.com/jfmengels/node-elm-lint), which has instructions on how to install it. This will allow you to lint your whole project.
The preferred method, if you have `Node.js` and `npm` installed, is to use [`elm-review`](https://github.com/jfmengels/node-elm-review).
Also, you can try the online version [here](https://elm-lint.now.sh), where you can copy-paste your source code and see the linting errors.
```bash
# Save it to your package.json, if you use npm in your project.
# This is the recommended way.
npm install elm-review --save-dev
# Install globally. This is not recommended.
npm install -g elm-review
```
Also, you can try the online version [here](https://elm-review.now.sh), where you can copy-paste your source code and see the review errors.
## Configuration
@ -35,9 +44,9 @@ Configuration is done via an Elm file. The benefit of having the configuration w
```elm
module LintConfig exposing (config)
module ReviewConfig exposing (config)
import Lint.Rule exposing (Rule)
import Review.Rule exposing (Rule)
import Third.Party.Rule
import My.Own.Custom.rule
import Another.Rule
@ -52,14 +61,14 @@ config =
```
You can get started with an empty configuration with the command line tool by running
`elm-lint init`, which you can then add rules to. Before you do, I suggest
`elm-review init`, which you can then add rules to. Before you do, I suggest
reading the rest of this document, but especially the section on
[when to enable a rule in your configuration](#when-to-write-or-enable-a-rule).
`elm-lint` does not come with any built-in rules. You can read why [here](https://github.com/jfmengels/elm-lint/blob/master/documentation/design/no-built-in-rules.md). You can find rules in the Elm package registry by [using `elm-search` and searching for `Lint.Rule.Rule`](https://klaftertief.github.io/elm-search/?q=Lint.Rule.Rule), and use by going to your lint directory and running `elm install` in your terminal.
`elm-review` does not come with any built-in rules. You can read why [here](https://github.com/jfmengels/elm-review/blob/master/documentation/design/no-built-in-rules.md). You can find rules in the Elm package registry by [using `elm-search` and searching for `Review.Rule.Rule`](https://klaftertief.github.io/elm-search/?q=Review.Rule.Rule), and use by going to your review directory and running `elm install` in your terminal.
```bash
cd lint/ # Go inside your lint configuration directory
cd review/ # Go inside your review configuration directory
elm install authorName/packageName
```
@ -70,7 +79,7 @@ section.
You can write your own rule using this package's API and
[`elm-syntax`](https://package.elm-lang.org/packages/stil4m/elm-syntax/latest/).
Check out the [`Lint.Rule`](./Lint-Rule) module for more instructions.
Check out the [`Review.Rule`](./Review-Rule) module for more instructions.
Here's an example of a rule that prevents a typo in a string that was made too
often at your company.
@ -80,7 +89,7 @@ module NoStringWithMisspelledCompanyName exposing (rule)
import Elm.Syntax.Expression exposing (Expression(..))
import Elm.Syntax.Node as Node exposing (Node)
import Lint.Rule as Rule exposing (Error, Rule)
import Review.Rule as Rule exposing (Error, Rule)
-- Create a new rule
rule : Rule
@ -118,9 +127,9 @@ expressionVisitor node =
Then add the rule in your configuration:
```elm
module LintConfig exposing (config)
module ReviewConfig exposing (config)
import Lint.Rule exposing (Rule)
import Review.Rule exposing (Rule)
import NoStringWithMisspelledCompanyName
@ -140,14 +149,14 @@ rule?" be "no". I think that the ratio that compares the rule's value to the
nuisances it will cause should be very high, and it is often hard to foresee
all the nuisances.
Linting rules are useful when something must never appear in the code. It gets
Review rules are useful when something must never appear in the code. It gets
much less useful when something should not appear only 99% of the time, as there
is no good solution for handling exceptions (`elm-lint` doesn't offer an option
is no good solution for handling exceptions (`elm-review` doesn't offer an option
for disabling a rule locally, see why [here](#is-there-a-way-to-ignore-an-error-or-disable-a-rule-only-in-some-locations-)).
If you really need to make exceptions, they should be written in the rule itself
or defined in the rule's parameters.
First of all, if you have never encountered a problem with some pattern before,
First of all, if you have never encountered a problem with a pattern before,
then you probably don't need to forbid it. There are chances the problem will
never occur, and writing the rule is a waste of time. Or maybe when using it,
you will find that the pattern is actually not so bad at all and that there are
@ -174,8 +183,8 @@ suggested way, probably making the code worse than before.
Some rules might suggest using more advanced techniques to avoid some pitfalls,
and this might make it harder for newcomers or beginners to get something done.
When enabling this kind of rule, make sure that it is helpful enough to unblock
users, otherwise this can frustrate and/or block them.
When enabling this kind of rule, make sure that the message it gives is helpful
enough to unblock users, otherwise this can frustrate and/or block them.
When wondering whether to write or enable a rule, I suggest using this checklist:
- I have had problems with the pattern I want to forbid
@ -204,11 +213,11 @@ There is none at the moment, for several reasons:
- If there are some rules that you really want to enforce because you want to
create additional guarantees in your codebase, and it is possible to ignore it,
then you will want a second system to ensure those rules are never ignored.
- When people encounter a linting error, quite often they will try to disable
- When people encounter a review error, quite often they will try to disable
it by default, because they don't agree with the rule, or because they think
they will fix it later or not at all. Just like we learned with the compiler
errors, some problems require us to do some additional work for good reasons,
and I think this should apply to errors reported by `elm-lint` too. Obviously,
and I think this should apply to errors reported by `elm-review` too. Obviously,
not being able to ignore a rule means that the bar to write or enable a rule
should be even higher.
- The more I think about it, the more I think that if you need to make an

25
documentation/design/no-built-in-rules.md
View File

@ -1,32 +1,31 @@
# Why there are no built-in rules in `elm-lint`
# Why there are no built-in rules in `elm-review`
`elm-lint` comes with no rules out of the box. It used to have some, but then I
`elm-review` comes with no rules out of the box. It used to have some, but then I
decided it was better to have them all extracted into different packages.
The main reason, is that if `elm-lint` had "core" rules, users would think they
The main reason, is that if `elm-review` had "core" rules, users would think they
are all best practices. I am of the opinion that in practice, any rule that
could be written can potentially be unwanted under certain circumstances, and
can therefore hardly be enforced all the time without more consideration.
`elm-lint` is plenty opinionated, but what counts as best practices in terms of
which linting configuration is not its concern, it is up to each user to decide
carefully whether or not to enable it. `elm-lint` will just provide the tools to
do so.
`elm-review` is plenty opinionated, but what counts as best practices in terms of
which configuration rules are enabled is not its concern, it is up to each user
to decide carefully whether or not to enable it. `elm-review` will just provide
the tools to do so.
If there are no core rules, then there are also no technical distinctions
between then and package rules. They all have access to the same tools, and
there is no rule that can do more than an other could.
A benefit of this, is that `elm-lint` will not need to have major version
changes when one of its rules gets updated or removed, because it doesn't have
one.
A benefit of this, is that `elm-review` will not need to have a major version
bump when one of its rules gets updated or removed, because it doesn't have any.
The inconvenience, is that if `elm-lint` happens to have a major version, which
The inconvenience, is that if `elm-review` happens to have a major version, which
I try to prevent as much as possible, all packages that depend on it (meaning at
least all packages with linting rules) will need to be updated and re-released,
least all packages with review rules) will need to be updated and re-released,
similar to what would happen when Elm gets a new (major or minor) version. Since
Elm doesn't (in practice) allow for duplicate dependencies with different major
versions, users would not be able to use the next version of `elm-lint` with the
versions, users would not be able to use the next version of `elm-review` with the
updated version of each package, until all packages are updated. Or they will
need to disable the rules from non-updated packages for a while.
This would happen even if there are built-in rules too, but more work would need

4
documentation/design/severity-levels.md
View File

@ -1,6 +1,6 @@
# Why are there no severity levels in `elm-lint`?
# Why are there no severity levels in `elm-review`?
In almost all linting tools I have seen ([here is a nice list of linters](https://github.com/caramelomartins/awesome-linters)), there is a concept of severity levels.
In almost all similar tools (linters, code checkers, ...) I have seen for other languages ([here is a nice list of linters](https://github.com/caramelomartins/awesome-linters)), there is a concept of severity levels.
In the example of [`ESLint`](https://eslint.org), there are three:
- "error": If an error with this severity level is found, it is reported and the CLI exits with a non-zero status.

6
documentation/design/test-module.md
View File

@ -1,6 +1,6 @@
# Design goals for the `Lint.Test` module
`elm-lint` comes with a testing library under `Lint.Test`, which allows you to test the linting rules you write using [`elm-test`](https://github.com/elm-community/elm-test). It comes with a guide explaining how to write tests efficiently.
`elm-review` comes with a testing library under `Lint.Test`, which allows you to test the rules you write using [`elm-test`](https://github.com/elm-community/elm-test). It comes with a guide explaining how to write tests efficiently.
`Lint.Test` has been created around several goals:
- Help developers with great test failure messages
@ -17,7 +17,7 @@ For this reason, custom error messages were mandatory. And since we are used to
When making a rule, there are plenty of things to test, and when left to developers with a general testing framework, a lot of them end up being ignored. Testing only that a rule reports an error for a given source code without checking the actual error information is a test that is likely to keep succeeding for the wrong reasons.
Because I think that all the information in an error is important, the test module forces you to test them all. Following are the element that `elm-lint` forces you to test.
Because I think that all the information in an error is important, the test module forces you to test them all. Following are the element that `elm-review` forces you to test.
### Error message and details
@ -39,6 +39,6 @@ This is usually a pain to test because it requires giving it a range (of the for
If (and only if) the error provides fixes, the test module requires you to provide what the code should look like after fixes have been applied. If the fix is invalid because it is malformed, doesn't make any changes or makes the code not syntactically valid, then the test will fail.
(In those situations, the fix is not presented to the user in the CLI)
When the user encounters an error and it provides fixes, `elm-lint` will mention them a fix is available, without checking that the fix is valid, because I estimate that trying out each change is probably too expensive in terms of performance. When running `elm-lint` with `--fix`, fixes will be evaluated and will be ignored (not presented to the user) if they are found invalid, which is not a great experience for them. `elm-lint` will warn about fixes that could not be applied, but having correct fixes in the first place will make for a much greater experience for the user.
When the user encounters an error and it provides fixes, `elm-review` will mention them a fix is available, without checking that the fix is valid, because I estimate that trying out each change is probably too expensive in terms of performance. When running `elm-review` with `--fix`, fixes will be evaluated and will be ignored (not presented to the user) if they are found invalid, which is not a great experience for them. `elm-review` will warn about fixes that could not be applied, but having correct fixes in the first place will make for a much greater experience for the user.
Not testing the fixed code means the user is more likely to see false fix positives, and to get valid but incorrect fixes.

0
documentation/images/elm-lint-report.png → documentation/images/elm-review-report.png
View File

Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 108 KiB

After

Width:  |  Height:  |  Size: 108 KiB

4
elm.json
View File

@ -1,6 +1,6 @@
{
"type": "package",
"name": "jfmengels/elm-lint",
"name": "jfmengels/elm-review",
"summary": "An Elm source code linter, to add additional guarantees to your project.",
"license": "BSD-3-Clause",
"version": "4.1.1",
@ -19,4 +19,4 @@
"stil4m/elm-syntax": "7.1.0 <= v < 8.0.0"
},
"test-dependencies": {}
}
}

2
package-lock.json generated
View File

@ -1,5 +1,5 @@
{
"name": "elm-lint",
"name": "elm-review",
"version": "1.0.0",
"lockfileVersion": 1,
"requires": true,

12
package.json
View File

@ -1,26 +1,26 @@
{
"name": "elm-lint",
"name": "elm-review",
"version": "1.0.0",
"description": "An [Elm](http://elm-lang.org/) linter written in Elm. Get your code from correct to better. You can learn about the API and the rules it provides on the [package documentation](http://package.elm-lang.org/packages/jfmengels/elm-lint).",
"description": "An [Elm](http://elm-lang.org/) linter written in Elm. Get your code from correct to better. You can learn about the API and the rules it provides on the [package documentation](http://package.elm-lang.org/packages/jfmengels/elm-review).",
"main": "index.js",
"directories": {
"example": "example",
"test": "tests"
},
"scripts": {
"test": "elm make --docs=elm-stuff/docs.json && elm-format src/ --validate && elm-test && elm-lint src/ tests/ demo/ lint/ && elm-xref",
"test": "elm make --docs=elm-stuff/docs.json && elm-format src/ --validate && elm-test && elm-review src/ tests/ demo/ review/ && elm-xref",
"test:watch": "npm run test -s -- --watch"
},
"repository": {
"type": "git",
"url": "git+https://github.com/jfmengels/elm-lint.git"
"url": "git+https://github.com/jfmengels/elm-review.git"
},
"author": "",
"license": "ISC",
"bugs": {
"url": "https://github.com/jfmengels/elm-lint/issues"
"url": "https://github.com/jfmengels/elm-review/issues"
},
"homepage": "https://github.com/jfmengels/elm-lint#readme",
"homepage": "https://github.com/jfmengels/elm-review#readme",
"dependencies": {
"elm": "^0.19.0-no-deps"
},

10
lint/LintConfig.elm → review/ReviewConfig.elm
View File

@ -1,12 +1,14 @@
module LintConfig exposing (config)
module ReviewConfig exposing (config)
{-| Do not rename the LintConfig module or the config function, because
`elm-lint` will look for these.
{-| Do not rename the ReviewConfig module or the config function, because
`elm-review` will look for these.
To add packages that contain rules, add them to this lint project using
To add packages that contain rules, add them to this review project using
`elm install author/packagename`
when inside the directory containing this file.
-}
import Lint.Rule exposing (Rule)

0
lint/elm.json → review/elm.json
View File