☂️ Linting

[sebmck]

The current area of focus for Rome is linting. This is the path of least resistance for finishing a part of Rome that can be used most easily.

With diagnostics #3 the lints that Rome can produce will be more detailed than any other existing JS linter. The most difficult part of this will be editor integration and writing a set of lint rules.

One large question is how we handle configuration. Are we going to take an opinionated stance and have some lints non-configurable? Will we have plugins?

[slobo]

Whether opinionated or fully configurable, it would be great to have ability to disable linter and/or prettifyer on blocks/sections of code - for those situation where a human believes they know better.

Something to this effect (excuse the unrealistic examples).

Sample Syntax:

// @linter-off rule1 [, rule2,...]: Reason why we need to disable listed rules

/* section of code that linter ignores */

// @linter-on

Examples:

• Turn off a particular linter rule on a block of code

// @linter-off prohibit-vars: Some lame excuse why I really need a var here
var blah;
// @linter-on

• Turn off a particular linter rule on a single line

var blah; // @linter-off prohibit-vars: because

• Similar for auto-formatter

// @formatter-off: I want my custom alignments, dammit 

fooBarnicate( 'hi',    'world' );
fooBarnicate( 'quick', 'fox'   );

// @formatter-on

Idea being that escape hatches are provided, but there is a sufficient friction in writing annotations and the extra line-noise they introduce hopefully makes people reach for them only when really justified.

Hopefully not a wrong place to voice this?

[sesam]

IMHO: Have good defaults, and make it slightly painful to disable the linter for a line. Add good linting explanations that link specifically to the linting repo....
This ensures that linter-rage :tableflips: reactions get targeted at the linting repo, and then just have chat-bots attend the issues there. The bots can be trained at calming people down and telling them their way is probably the best, but the linting rules are there to help those less experienced to better understand the code and avoid producing hard to grasp structures and bug magnets. 😄 And that might even be true.
For fixing any early and eventually bad linting decisions: this is a monorepo with no external dependencies so we're always just ONE nicely motivated PR away from fixing that cleanly and nicely. Though it would probably also need to include a codemod-style automatic code upgrader, to not put-off contributors who have unmerged branches.

[jloleysens]

One large question is how we handle configuration. Are we going to take an opinionated stance...

I think the extendability of formatting linting should be limited. Perhaps a small set of options could be exposed in config (a starting point could be a subset of those Prettier exposes https://prettier.io/docs/en/options.html, like newline and semicolon).

It's just one less thing to have to think about - which complements the goals of this project. We can take a cue from go fmt here, IMO.

Opening up to extendability will also require a ton more design decisions to be made. Perhaps it is something that can be iterated on later (extensibility)?

[chadhietala]

While I agree with @jloleysens, one thing I think is fairly common in large codebases is introducing lint rules as a means to migrate away from specific APIs in addition to codemods. However, if there is the capability participate in compilation through plugins one could create their own application specific linting tools based on the Rome AST.

[Aghassi]

I'm torn on escape hatches. They are needed, but also I've seen them abused by devs who don't care. I think the main question for that is what are we trying to achieve with listing rules. Are they style rules for formatting, or are they actual bug prevention rules? Style rules can always be overridden, but some bug prevention stuff I feel pretty strongly should not have an out.

As for plugins, it is a double edge sword because the more plugins you have, the harder it is to update the core utility if you have to wait on the plugin to also update. I think to @chadhietala point, there should be some way to lint and run codemods on larger code bases through special rules. I just don't know if that warrants a "plugin" framework. Not to mention that there can be friction when updating plugins if they change rules that cause your code to format differently.

I'd also like to callout that it is harder to adopt rules in a larger codebase because normally it is "all or nothing". The only thing I have seen that gets around this is https://www.kadamwhite.com/archives/2017/progressive-linting, but it isn't super ideal as it means areas of code will remain ignored until they are touched. It would be interesting to see if there is a way to take this into consideration when expanding the linting tool of Rome.

[arvigeus]

I think instead of error/warnings like eslint, Rome should categorize code based on levels, example:
1: Errors. Reserved only for invalid JS code, things that will throw exceptions when executed.
2: JS quirks. Things that JS allows, but will most likely produce unexpected result. Examples: duplicate object keys, using variables before defining.
3: Ambiguous code: Things that are possibly wrong, but might also be fine, like parseInt(x). Should allow inline disable.
4: Preferences: Any coding conventions that should be followed by the developers, like disallowing ternary. Should be configurable, all rules disabled by default, and should allow inline disable.
5. Style: Basically what Prettier does, but a bit more extensive. In other words, eslint --autofix-eable rules like prefer-ternary. Should allow configuration. Single/double quotes config should be here.

By default the max level displayed should be 4 - styling issues should not be reported since Rome will take care of them at some point (save or rome --autofix), and they are too much noise that most people usually don't care about.

Having such levels or categories will allow richer editor experience: not just red or yellow underlines, but other colors as well to indicate different type of problems.

4 and 5 are very close to each other, the only difference is 4 rules cannot be autofixed.

[sebmck]

Dumping some of my brief thoughts here:

• No formatting options. Zero. None. Prettier initially tried to do this but caved under demands for things like an option for quote type etc. If you aren't happy with the defaults that Rome provides then you can use prettier or another tool.
• AST transforms for prettification. This is a hard stance that prettier has taken where they don't want to modify the AST in any way. However there are a variety of transforms you can apply that retain all the semantics but make the code more readable. These can be things like flipping conditionals, pruning dead code/empty blocks etc.
• Suppression comments must be specific. You must explicitly to all rules you want to disable for a particular line. Right now with other linters it's possible, and common practice, to suppress all lints for a specific line. Any future lint rules wont effect previous suppressions, this is bad!
• Autofixing AST rules. Rome allows you to write lint autofixes with an AST transform. This is more powerful than ESLint which is just string based. With this capability we should be able to have autofixes for a majority of errors.

[jonstuebe]

Love all of those @sebmck. If we learned one thing from the rails world, it's setting up conventions that are inmovable. Allows us all to have consistent projects and stop any/all arguments. If you want things to be ultra configurable, then the individual tools exist (i.e. prettier, eslint, etc).

[ava-gunn]

PHP's static analysis tools have started to introduce the concept of a baseline file which suppresses errors for existing code but not for new code.

I've found this helps limit the proliferation of suppression comments while at the same time providing just enough friction to discourage overuse.

[dwelle]

I'm all for zero formatting options. I can get over anything even if it deeply offends my aesthetic sensibilities, as long as we're saved from styleguide wars.

That said, it might not be obvious where to draw the line between formatting and semantic/logic/error lint rules, such as prefer-const. One would say it's formatting, I'd say it's code-complexity issue.

Oh noes, are we back to styleguide wars?

[jahzielv]

I'm also on board with no (or very limited) options. One of the things I love most about Go is how strong its opinions are, even about linting/formatting. It might ruffle some feathers, but it's something that people can learn to live with (I learned to live with tabs in Go 😖).

[zubhav]

I agree with having a set of configurations that come out of the box. At this initial stage, it is probably a good idea to leverage this feature to show that Rome will be somewhat opinionated. However, as mentioned above by a few, there is reason to have flexibility in such enforced rules. Developers shouldn't have to battle against the default lint rules. They should be guidelines to maintain a standard of code. Rome should define it's base rules which can be overridden by an alternative.

As for things like escape hatches, we should be trying to minimise occurrences of this with a sensible approach that is in-scope of this project. The idea proposed by @slobo is worth exploring but I'm not sure whether the friction it causes will actually resort in escape hatches being used less.