Bug: semi option omitLastInOneLineBlock does not work for one liner class blocks

[martian17]

Environment

Node version: v18.14.1
npm version: pnpm 7.29.1
Local ESLint version: 8.37.0
Global ESLint version: N/A
Operating System: Ubuntu

What parser are you using?

Default (Espree)

What did you do?

Configuration

// .eslintrc.cjs
module.exports = {
    extends: [
        "eslint:recommended"
    ],
    env: {
      browser: true,
      node: true,
    },
    parserOptions: {
        ecmaVersion: "latest",
        sourceType: "module"
    },
    rules: {
        "semi": ["error", "always", {
            "omitLastInOneLineBlock": true
        }]
    }
}

// index.js
export class SomeClass{
    logType(){
        console.log(this.type);
    }
}

export class Variant1 extends SomeClass{type=1}
export class Variant2 extends SomeClass{type=2}
export class Variant3 extends SomeClass{type=3}
export class Variant4 extends SomeClass{type=4}
export class Variant5 extends SomeClass{type=5}

What did you expect to happen?

No errors

What actually happened?

Here is the error

root/index.js
   8:47  error  Missing semicolon  semi
   9:47  error  Missing semicolon  semi
  10:47  error  Missing semicolon  semi
  11:47  error  Missing semicolon  semi
  12:47  error  Missing semicolon  semi

✖ 5 problems (5 errors, 0 warnings)
  5 errors and 0 warnings potentially fixable with the `--fix` option.

Participation

• I am willing to submit a pull request for this issue.

Additional comments

If this is the expected behavior, I'd still love to have an option to enable checks for single line classes.

[martian17]

I confirmed that this error goes away after editing line 324 of /lib/rules/semi.js to match "ClassBody" in addition to "BlockStatement".
https://github.com/eslint/eslint/blob/main/lib/rules/semi.js#L324
before:

            if (parent.type === "BlockStatement") {

after:

            if (parent.type === "BlockStatement" || parent.type === "ClassBody") {

To elaborate on why the parent type should be "ClassBody", it is to make the behavior consistent with the existing semi rule implementation and match exactly the part enclosed by the opening and closing curly braces including the braces themselves. In the AST, class is represented by "ClassDeclaration" and its child "ClassBody". "ClassDeclaration" encompasses the entire class statement, and "ClassBody" represents the part enclosed by curly braces.

Should I make a pull request with this change? Or should this behavior be added as an option instead?

[mdjermanovic]

Hi @martian17, thanks for the issue!

I just checked PR #14945 where the semi rule was updated for class fields, and there was no discussion about the omitLastInOneLineBlock option. I think we didn't consider updating this option because class bodies are not "blocks". On the other hand, function bodies aren't blocks either (although they're BlockStatement nodes in ESTree AST, they are generally not considered to be "blocks") but this option does apply to them, so it might make sense to apply it to class bodies as well. However, as the omitLastInOneLineBlock option not only allows the omission of semicolons but also disallows them, updating this option would now introduce new warnings so it looks like a breaking change to me. I would support adding a new option omitLastInOneLineClassBody for the sake of completeness of this rule. Curious what other team members think about this.

[nzakas]

I would support adding a new option omitLastInOneLineClassBody for the sake of completeness of this rule. Curious what other team members think about this.

Makes sense to me. @martian17 do you want to submit a pull request for this?

[nzakas]

Looks like @snitin315 assigned this to himself. In the future, please leave a comment saying that you're working on it for clarity.