javascript: [base/rules/es6.js] Why are all rules on error?
Most of these rules need to be changed from error [2,...
to warn [1, ...
…
https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/es6.js
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Reactions: 13
- Comments: 28 (2 by maintainers)
They are errors intentionally. Warnings are mostly useless, since they are easily ignored. If it’s not an error, it’s not enforced.
Ok, but I think your mixing personal problems with your software… If users want to have a policy of no warnings they can do that. Your project would be more useful if it benefited from the warning feature.
@BohdanTkachenko all rules are critical, and ALL rule violations should break the build, of any kind.
If you disagree, you’re welcome to fork and/or override anything you like and make them into warnings.
I also think that it could be better if not all rules to be like errors. My point is following: I’m using eslint in Atom to see if I have any errors in my code and also I use Webpack with eslint-loader and NoErrorsPlugin, so when there is any ESLint error in my code - it does not compiling, which is good, because it saves a lot of time when it detects an error that makes unable for my code to run. But the thing is that sometimes when developing I need to insert console.log, or I need to quickly add an element to array to test something (and sometimes I’m doing it too quickly and not adding comma in the end of a line) and in this case Webpack will just not compile my code because ESLint tells that there are errors. But from my point of view they are more likely warnings, not errors, because code will work, but it will be just not perfect at this point.
I think that it would be much better if all critical rules which can detect places that can break something to be like errors, but for other ones that are more like stylistic - everything that is not good to commit, but that will work - to have warning level, so it will more like informative.
BTW, for CI systems you can set --max-warnings 0 flag for eslint and it will fail build even if there are any warnings detected, so it is possible setup it correctly when building project, but it is not so usable for development in some cases. What I’m doing right now is just redefining some rules in my local .eslintrc, setting some rules to warning instead of error
The thing is that in my case I have webpack plugin that kind of saves some time for me by checking if there are any critical ESLint errors (for example, wrong syntax), so I know about an error before I open my app in a browser.
Btw, also another point. When I edit code and my code editor lints file and reports some error in yellow color, I visually see that it’s something not so important, and I can come back to fix it a bit later, if I’m debugging something. But if I see red color, then it’s something that can break build or cause some problems even when debugging, so I need to fix red ones immediately to be able to continue to debug.
Probably it is just easier just not to use this NoErrorPlugin for Webpack (it’s not so important, just makes life a bit easier), but I just was interested, maybe you and other maintainers of this repository find my point of view convincing and I can work on PR.
With all the warnings set to error, I can’t see errors. I’m viewing other peoples projects and code so it is not possible to fix them. It makes the most since to put any thing that does not break at runtime on warning and anything that will create an error at runtime on error.
I’m saying it should be granted the same level of severity as any other error. Marking some things as merely “warnings”, even if they fail the build, encourages devs to think of them as somehow less severe or less important - which they are not.
That’s commonly and easily achieved with an editor plugin - there’s no need to put that in webpack.
In general though, you shouldn’t continue until you’ve added the semicolon on line 56. The idea that some linter errors are “less important” is the one I have a problem with. A missing semicolon should break your entire build and halt everything just the same as a syntax error.
I agree with you but I’m much happier now that I don’t have red errors in Atom (using a fork) 😃
Fundamentally I think making all rules errors was the wrong decision spurred on by wanting to make the build fail.
Especially where the lint errors complain about minor things such as indentation, whereas in a compiled language you’d of course expect a type error to fail the build.
@ljharb there is a difference between breaking a build and cleaning up your code before committing it. I also hit this exact issue recently and it’s doing my head in (seeing red errors in Atom alongside Flow errors and not knowing which one is which). Setting --max-warnings 0 to treat warnings as errors for CI seems to be most sensible. The issue is not being able to quickly write code and then cleanup, because everything is red until you write perfect code.
Indentation inconsistencies should fail the build. If a coder was sloppy about indentation, who knows what other (often unlintable) bugs they were careless enough to let slip through.
We have c++ coders using different styles on our project too (writing JavaScript)… It is not a big deal to me to read their code. I do like to spot errors though, I am modifying that code.
Yeah, I know what do you mean. I also like an idea that when you build a project, all rules are important and even silly console.log can make problems in old IE, for example. But I mean that in most cases when debugging your code, some of the rules can be ignored because they’ll not cause any problems in your environment.
But I rather agree with you, because each developer or project has different environments and it is not possible to tell in which environment which rules are critical and which are not.
Probably the best solution is to create a new config which extends this config, but just overrides error levels for some of the rules and use this config only in some projects where it’s acceptable.
Thanks for your comments!
In that case, don’t you simply ignore the linter warnings? The linter should run pre-push, but it’s kind of silly to have your local dev build process fail based on linter warnings imo.
@ljharb I completely agree with you and code that does not pass linter should never be commited to GIT and it should always break build and this project is awesome in this task - it really helps to find bad code before it is commited. I mean only the situation when you work with code locally and you need something like console.log for debugging only. Setting different error levels for different rules will just allow to setup linter in different ways, but it still will break builds with bad code.
I use this work around to move the error level to warning. I’m sure it mislabels things, but in my coding I don’t see too many warnings (after the patch) that should be errors so this works great for me. It does not effect other users.
Style issues may not break on runtime, but in an interpreted language (without a compiler to catch many of these kinds of errors at build time), it’s critical to catch these errors at build time. Everything we consider enforceable here is just as dangerous in a codebase - whether it interferes with parsing or not.
Linters should be employed at a project level, not across projects, so I’m confused why you’re seeing errors in other people’s projects.