denoland / deno_lint

Blazing fast linter for JavaScript and TypeScript written in Rust
https://lint.deno.land/
MIT License
1.54k stars 171 forks source link

Also lint CSS #835

Open jaydenseric opened 3 years ago

jaydenseric commented 3 years ago

It would be great if Deno could also lint CSS in .css files, both via the deno lint CLI but also via the Deno LSP, which at the moment is capable of formatting css syntax.

The default CSS lint rules should basically just validate the CSS; there should be no formatting related rules as that's handled by Deno fmt.

Ideally there would also be rules to validate and auto fix CSS vendor-prefixes, against browserslist config.

In the meantime, I'm not sure the best way to handle linting of CSS in a Deno project without introducing a package.json file with dev dependencies such as stylelint installed and run via npm 🤮

kitsonk commented 3 years ago

Because Deno doesn't handle CSS natively, or even import it, it seem like it is a scope issue. It is a lot more than waving a magic wand and linting CSS. It requires a parser and an opionated set of style rules, neither one Deno has any opinions about.

Personally I would be opposed up until the point where Deno imports and processes CSS, which with the current state of the discussion with TC39 and the wider platform, that won't be anytime soon.

bartlomieju commented 3 years ago

I agree with Kitson. Deno is completely agnostic of CSS and can't use it in anyway.

Personally I would be opposed up until the point where Deno imports and processes CSS, which with the current state of the discussion with TC39 and the wider platform, that won't be anytime soon.

Agreed, this request is currently out of scope for deno_lint and I'm not keen on adding CSS infrastructure.

jthegedus commented 3 years ago

I came here not for CSS, but expecting deno lint to support the same set of files that deno fmt supports. I understand these are built on separate underlying rust infra, however there is no clear distinction of the goals with these two cmds, one would expect their goals/coverage to align.

bartlomieju commented 3 years ago

I came here not for CSS, but expecting deno lint to support the same set of files that deno fmt supports. I understand these are built on separate underlying rust infra, however there is no clear distinction of the goals with these two cmds, one would expect their goals/coverage to align.

So you expect deno_lint to lint Markdown and JSON/JSONC files?

kitsonk commented 3 years ago

What rule set would you propose for linting Markdown and JSON/JSONC?

jaydenseric commented 3 years ago

What we don't want, is the linting wheels to be reinvented again and again like the situation the npm ecosystem is in. To lint JS files via ESLint, you need to install the dev dependency, setup it's config, setup the ESLint CLI for the GitHub Actions CI, and find an editor plugin. Then if you also need to lint CSS, you have to install the stylelint dev dependency, setup it's particular config, setup it's CLI for the GitHub Actions CI, and find a separate editor plugin. And so forth for any other syntaxes, although JS and CSS are the big ones.

From the OSS side of things, ESLint, stylelint, and other lint tools are having to design, build, and maintain separate systems for config, CLI, reporting, etc. again and again. General problems solved by one tool (e.g. sharable configs) might take years to be solved for others. A lot of duplicated and wasted effort.

Configs for the various tools end up spread across multiple files with different systems for enabling rules, ignoring files, etc. The ignore comments are stylistically different between tools. You end up with complicated config to get CSS in JS to be linted with stylelint, and JS code examples in markdown to be linted with ESLint, or markdown/CSS/JS in JSDoc fenced code examples to get linted by the right tools. Deno could holistically lint all these mixed syntax contexts. Lint tool CLI's put out slightly different looking reports, and take different args that need to be learned and remembered. Some editors don't have a stylelint extension at all, so you're just shit out of luck in that case and can only use the CLI.

It would be amazing if the Deno CLI and language server could allow linting plugins for any of the Deno formattable/language server languages to be installed. Only one Deno editor extension using the Deno language server needs to be installed. Only one Deno lint CLI needs to be learned, with no installation needed since it's out of the box. Setup one lint config file for all syntaxes. Setup GitHub actions CI once, and then Deno lint plugins and config can be chopped and changed easily at will.

An amazing amount of teams don't do any linting at all, because it's all too fiddly. Or they lint some syntaxes, but not others (e.g. people bother with ESLint but neglect CSS). Deno has an opportunity to get this right from the start, or at least provide a super fast common Rust infrastructure and unified config system for the community to work with and publish third party plugins.

What rule set would you propose for linting Markdown and JSON/JSONC?

For example, linting certain JSON files according JSON schemas.

bartlomieju commented 3 years ago

@jaydenseric I understand your sentiment and indeed linting codeblocks in Markdown files could be done from our point of view (similar to how deno fmt can format codeblocks in .md files).

I'm not so sure about validating JSON files against JSON schemas - best to open a separate issue about it.

As for CSS - this would be a huge undertaking and we currently don't have any infrastructure to do that, plus Deno doesn't recognise CSS files in anyway. Once support for standardised CSS modules lands we could reevaluate.

We do have a plan to make deno lint more configurable via a config file, but I must warn you, we do not want to go ESLint's way where every rule has several settings that can be tweaked. Plugin support is nascent at the moment, but the same rule would apply regarding configuration of rules.

kitsonk commented 3 years ago

For example, linting certain JSON files according JSON schemas.

How do you propose to associate a specific JSON file with a JSON schema?

bartlomieju commented 3 years ago

For example, linting certain JSON files according JSON schemas.

How do you propose to associate a specific JSON file with a JSON schema?

https://json-schema.org/learn/getting-started-step-by-step - using $schema and $id fields. Though in https://github.com/denoland/deno/issues/11885 @dsherret suggest we shouldn't go with this solution for Deno's config file.

kitsonk commented 3 years ago

Yeah, I know about that. My point is "how deep is the rabbit hole"... editors already solve this problem. For example, vscode allows registration of file patterns to schemas and enforces the JSON schema. This is how we support format enforcement and suggestions in import maps in vscode_deno.

It requires lots of configuration though, and is not something well suited for the Deno CLI. Editors and their clients solve it better.

jaydenseric commented 3 years ago

Deno could show lint errors/warnings for import map JSON files, and Deno config files (whatever form they end up taking). For other third-party tool specific JSON files, schemas could be associated to specific files via user Deno lint config if deno ships a configurable JSON schema rule out of the box, or just by a third party Deno lint plugin. For example, a user could install a third-party Deno lint plugin for linting package.json fields.

I'm not in urgent need of JSON schema linting, it's just a random example off the top of my head of JSON file linting (beyond basic JSON syntax validation) users might want. It's pretty useful when editing VS Code settings files:

Screen Shot 2021-09-08 at 3 21 31 pm

I can't really live without CSS linting, so for now I've hacked together a Deno based CLI that can validate all .css files in a project using the online W3C CSS validator:

Screen Shot 2021-09-04 at 10 39 52 am

It was actually very fun and elegant to code (< 100 lines of TS), since Deno has such great std APIs for ANSI formatting, expanding file globs, etc. It's not a great long-term solution though:

jthegedus commented 3 years ago

So you expect deno_lint to lint Markdown and JSON/JSONC files?

Well if there are opinions about how those files should be format then I expect there are pitfalls for writing them that can be linted?

How that looks, I do not know.

At a minimum, updating the documentation for each of the commands to list the file types they apply to by default would be helpful. (this would satisfy the catalyst for me commenting here)


My point is "how deep is the rabbit hole"... editors already solve this problem. For example, vscode allows registration of file patterns to schemas and enforces the JSON schema. This is how we support format enforcement and suggestions in import maps in vscode_deno.

Support for linting a file format that has a spec (eg JSON) and autocomplete of specific implementations of that spec are different IMO. EG: a .json file with a // comment is invalid and a potential rule for a linter to check.


On the further discussions, these are my thoughts & perspective:

Node.js expanded from just a server runtime for JS to being the toolchain for most JS development, both frontend and backend. From my perspective Deno has a clear goal to reduce the fragmentation of the current (JS) ecosystem by shipping a CLI with batteries included (lint, fmt, bundle, compile), so evolving to support format and linting of adjacent tooling warrants discussion. Whether or not it is a good idea will come from discussion.

Using Deno gives me significant vibes to Rome Tools in that:

Rome is a linter, compiler, bundler, and more for JavaScript, TypeScript, JSON, HTML, Markdown, and CSS.

It currently supports Parse, Format & Lint of JS, TS, JSX, JSON, RJSON, HTML, CSS, Markdown.

Additionally, eslint, which is the tool with which feature parity is measured has plugins for css, html etc, so it is certainly possible to support linting with those rulesets. Again, whether or not it is a good idea is another question of which the answer comes from discussion and prototyping.

jthegedus commented 3 years ago

Futhermore, would deno bundle ever support bundling HTML, CSS & JS/TS, probably not, that doesn't make much sense. But, should the default formatter of the Deno ecosystem support formatting and linting these auxiliary file types, perhaps.

kitsonk commented 3 years ago

Let's define a few things here, so we are clear what we are talking about.

We already provide syntactic diagnostics for import maps and tsconfig files from the CLI, and will provide them for the config file as we add features. Providing syntactic diagnostics for JSON files in an editor is best left out of the scope of the Deno language server in my opinion. Providing better semantic diagnostics is something we should consider and do IMO. The vscode_deno extension already provides some semantic/syntactic diagnostics for import maps via the ability to map files to JSON Schemas. I prefer that approach as it saves from bloating Deno CLI with a JSON Schema validator. Adding on more semantical errors, like resolving specifiers in the map, is the domain of Deno language server though.

There is no "linting" of JSON files above that of formatting, which is already provided.

Syntactic diagnostics for JSON, in the base sense of "is it valid JSON" would come into scope when JSON modules are supported by Deno. Enforcement of schemas though is not something IMO we should get into with Deno built into the runtime. It really is a concern of user code to enforce such semantics as they see fit.

Applying syntactic diagnostics to code blocks might make sense, but as mentioned that isn't the "Also lint CSS" issue. We have experimented with this with deno test and that continues to feel like the right way to expose this. Also we have found that applying semantic or linting diagnostics though would likely be a problem, because these are often code snippets and not fully expressing everything, leasing to lots of false positives to the point of being unusable.

Outside of code blocks and formatting, syntactic diagnostics generally don't exist for markdown and linting rules seems really like something that Deno should get involved with.

As discussed CSS semantics and syntactics might be in scope when and if CSS imports become part of JavaScript. Linting is still a complicated thing that could only be considered well beyond that point.

jthegedus commented 3 years ago

I think we agree in the low value that a JSON lint would provide when a formatter would already bubble some of the information to the user.

Having said that, the ruleset is rather small as shown in eslint-plugin-json and would surface the issues to those who expect to find them in a Linter. The desire for this is shown with eslint-plugin-json having 250k downloads/week on npmjs (a crude metric). Similar metrics exist for the HTML and CSS eslint plugins.

Mapping to JSON schemas was not my goal of raising JSON, I agree that the featureset of JSON schemas is another issue entirely and personally think it should live at the extension layer as mentioned.

JSON was merely an example of the three (JSON, HTML, CSS) file formats that Rome has committed to delivering linting for which Deno has yet to commit specific support for.

I landed here because I wanted to know what Deno could lint. I had an idea of what it could format and was surprised they differed. I see other tools supporting both formatting and linting in unison for file types, even if there is overlap between lint & format, or the ruleset is trivial, or the ruleset a stricter subset of an already supported ruleset.

I will let @jaydenseric resume this conversation specifically about CSS.

kitsonk commented 3 years ago

JSON was merely an example of the three (JSON, HTML, CSS) file formats that Rome has committed to delivering linting for which Deno has yet to commit specific support for.

Deno and Rome's design goals are quite a bit different.

The desire for this is shown with eslint-plugin-json having 250k downloads/week on npmjs (a crude metric).

Or someone has included it in an opionsted stack and 250k people a week are downloading something they aren't aware of and don't consciously use, in the never ending bloat that is the npm black hole?

jthegedus commented 3 years ago

Or someone has included it in an opionsted stack and 250k people a week are downloading something they aren't aware of and don't consciously use, in the never ending bloat that is the npm black hole?

Yes, I stated this was a crude measurement.

Deno and Rome's design goals are quite a bit different.

Yes, Rome will utilise Node and it's package ecosystem and so it's goals are different to Deno. The tools undoubtedly have overlap in the "batteries-included" aspects (linting, formatting etc) of which I was trying to discuss. If the minds behind Rome see a purpose in including the aforementioned, then I think that warrants sincere consideration.

kitsonk commented 3 years ago

Rome is development tooling to develop web applications. Deno is a web platform runtime. They are very distinct things.

jthegedus commented 3 years ago

I understand that, I feel you are specifically ignoring the actual points I am trying to raise.

dsherret commented 3 years ago

This conversation on JSON and markdown (differences between deno fmt) feels off topic. Maybe there should be a separate issue?

RobertAKARobin commented 1 year ago

Linting CSS is really a solved problem, thanks to Stylelint. Instead of Deno reinventing the wheel I would prefer Deno just introduce a way to run Stylelint.

jrson83 commented 1 year ago

Linting CSS is really a solved problem, thanks to Stylelint. Instead of Deno reinventing the wheel I would prefer Deno just introduce a way to run Stylelint.

Yes, you cannot use the vscode stylelint extension with deno and for me it is also not working when installed stylelint globally with npm. It is really annoying. If you don't want to install stylelint with npm inside a deno project, the only way using stylelint is to install it outside the project folder and set a relative path in the extension stylelint path config.

RobertAKARobin commented 1 year ago

Sort-of related to https://github.com/denoland/deno_lint/issues/25, which concerns Eslint.