ESLint support

[imm]

Continuing the discussion started in #832

In my spare time, I look at ESLint rules for compatibility with Civet. So far, I have only looked through a few dozen, but it has already become clear that it may not be worth looking through all the rules to the end, even though this has its own benefits.

Most likely, the code that Civet produces does not satisfy only a few rules, and most likely, these are the rules from the Suggestions block. So my idea to write something like tests for each rule was, to put it mildly, not the smartest.

It seems to me that the code that can be passed into ESLint for testing is actually already written in Civet's own tests. After all, as far as I understand, there are potentially all the cases of code that Civet generates (excluding complex or missing cases). It's quite easy to write a script to check the code of all test cases for compliance with ESLint rules. In this way, a set of Civet rules for ESLint can be easily made, and those cases that do not allow full compatibility can be identified.

What do you think?

Regarding the empty catch block to support the corresponding ESLint rule, I looked at how to add a comment to it (discussed in issue #832) but decided not to do it for now, as it might be better done as part of the collective movement towards full ESLint support. After all, Civet generates empty blocks not only for the catch statement.

[imm]

Just out of curiosity, I added a check via eslint to the testCase function. Configuration used:

{
  extends: ["eslint:recommended"],
  parser: '@typescript-eslint/parser',
  plugins: ['@typescript-eslint']
}

The eslint:recommended ruleset was used. I've got following numbers of eslint errors:

rule	# of cases
no-undef	1659
no-unused-vars	759
no-constant-condition	36
no-extra-semi	27
no-const-assign	16
no-redeclare	15
no-cond-assign	13
no-import-assign	12
no-unreachable	5
no-sparse-arrays	4
no-empty	4
no-setter-return	3
no-useless-escape	2
no-debugger	2
no-unused-labels	1
no-mixed-spaces-and-tabs	1

I did it very clumsily and some tests that should have been skipped (using .skip) ended up in the statistics.

After looking at the results, I decided to analyze those tests where these rules were violated and share the results.

no-undef, no-unused-vars
These two rules cannot be tested in this way because almost every test case has an undeclared or unused variable.

no-constant-condition
This is generally the user's responsibility, maybe except for the loop loop, because loop is while (true) {}. It might be worth using the checkLoops option.

no-extra-semi
This rule is marked as depricated, but for some reason it is included in the set. According to my observations, Civet actively uses semicolons. This rule should probably be disabled.

no-const-assign
This rule is violated in tests for the auto-const, auto-let and auto-var directives. I didn’t quite understand what to do with this rule for the user. In theory, this is entirely the user's responsibility.

no-redeclare
This is also a feature of the tests and the responsibility of the user.

no-cond-assign
This rule did not work in many places before, since expressions were written in single parentheses instead of double ones. The tests currently use an old version (0.6.45) and the tests apparently contain outdated code, since the latest version of Civet puts additional parentheses for assignment in the conditions. Compare this and this. Maybe this rule is now fully supported?

no-empty
Violating this rule in tests is normal, but such an error can occur in empty catch. This rule has an option allowEmptyCatch. Currently the user can add a comment to an empty block using a semicolon before the comment.

no-import-assign, no-unreachable, no-sparse-arrays, no-setter-return, no-useless-escape, no-debugger, no-unused-labels, no-mixed-spaces-and-tabs
This is entirely the user's responsibility, I believe.

Of course, this is just a recommended set of rules from ESLint and there may be situations that are not described in the tests, but in my opinion, not much is needed to use Civet and ESLint together. For now it is enough to disable no-extra-semi, enable the checkLoops option for no-constant-condition and perhaps add an empty comment for the implicit catch block for the no-empty rule.

[edemaine]

This is great! Sounds pretty straightforward in fact.

We often preserve (lack of) semicolons from the input, except where they need to be added, but some generated code like variable declarations gets semicolons for safety. We could eventually try to improve the latter so no-extra-semi could work too (as it'd be a nice rule to encourage Civet style of no semicolons whatsoever).

I wonder if it would make sense for Civet to automatically turn eslint on/off for certain constructs. For example:

loop
  while false
    x
↓↓↓
while (true) { // eslint-disable-line no-constant-condition
  while (false) {
    x
  }
}

This way, user's while false will still trigger no-constant-condition, while loop does not, which I think is the spirit of the rule. Exact placement of disabling comment(s) to be determined.

This way, eventually, we might not need to disable any rules to use Civet + eslint effectively.

[danielbayley]

We definitely need an eslint-plugin-civet (-preset-?)

[edemaine]

Related: is there a way to directly lint .civet files with a custom parser/compiler/preprocessor? Probably we need to make a small tool here, but would make linting especially convenient. Perhaps the same package could provide presets. Probably part of the monorepo.

[imm]

Somehow this is done in typescript-eslint. This should probably be possible for Civet too. Being able to check Civet syntax directly would be a great feature. There is still not enough custom configurable prettier for the complete picture.

There is some documentation on how to write custom parser: https://eslint.org/docs/latest/extend/custom-parsers. My competence ended when the part about AST began.

As far as I understand, it is possible to create a parser not only for .civet, but also for test files. Perhaps the latter is even easier to do, since we can use a parser for typescript, pulling out only lines of code with the result of compilation.

For .civet a similar approach could be used since there is already a Civet → Typescript conversion. But I personally don’t understand how to map error positions back (Typescript → Civet). Apparently the same task is faced by the plugin for VS Code and work with AST will still be required here.

It’s interesting how this is done in Typescript, since the original ESLint rules were written for Javascript. It would be nice to take a peek in typescript-eslint.

[danielbayley]

Related: is there a way to directly lint .civet files with a custom parser/compiler/preprocessor? Probably we need to make a small tool here, but would make linting especially convenient. Perhaps the same package could provide presets. Probably part of the monorepo.

@edemaine See also https://github.com/antfu/eslint-ts-patch, for how it might enable something like eslint.config.civet…

[edemaine]

Update: I've made a basic ESLint plugin for Civet. Still need to add recommended configs, but the transformation of Civet files should be automatic during linting now.

[danielbayley]

I'm migrating to Biome now…

How big an effort would it be to support Civet with a Biome 2.0 plugin?

[edemaine]

Nice to hear that Biome is starting to add plugins! If I understand the current plugin support (see also umbrella issue biomejs/biome#2463), all that exists so far is analysis plugins; there isn't support for preprocessing other languages. (See in particular TypeScript discussion in biomejs/biome#1762)

So I'm afraid it's probably too early for see automatic Civet support in Biome. For now you'll probably have to compile to JavaScript (Biome doesn't support TypeScript?) and run Biome on the output.

[danielbayley]

Biome doesn't support TypeScript?

Sure it does.

For now you'll probably have to compile to JavaScript … and run Biome on the output.

Yeah I figured an initial version would likely have to take this approach… Would you hold out until official support for different parser/AST-based plugins? @edemaine

For reference, here is a list/roadmap of supported languages. YAML support for example, is already in progress…

[edemaine]

OK, weird, I'm not sure what biomejs/biome#1762 is talking about with respect to the typescript-eslint plugin; maybe Biome doesn't currently lint the type annotations, whereas typescript-eslint does?

The point is that I don't think we can currently add Civet support to Biome (without forking Biome), until they allow for custom languages. So if you want it sooner:

1. We could submit a PR to Biome that does it, either adding generic support, or adding Civet support when @danielx/civet is installed, say.
2. We could write a script that does the conversion and then calls Biome. Especially if Biome offers a JS API, this might not be too painful... So maybe we "just" need to fork the Biome CLI in some form?

Are you interested in working on either of these?