Rule to enforce semicolons after bound class methods

[rkrisztian]

Bug Report

• TSLint version: 5.7.0
• TypeScript version: 2.4.2
• Running TSLint via: CLI

TypeScript code being linted

This example may arguably be a bit too simple, but I've got real code like this.

class MyClass {
    	private myFunc =
    	    	    	([param1, param2, param3]) => param1 && && param2 && !param3;
        private myVar = 'test';
}

with tslint.json configuration:

{
        "semicolon": [
            true,
           "always"
        ]
}

Actual behavior

The error I get is Unnecessary semicolon for myFunc, but it does not feel natural at all to to remove the semicolon from there.

And if I make use of the relaxing option "ignore-bound-class-methods", then there is no more enforced consistency, i.e. either way is allowed.

Expected behavior

I expect my TSLint configuration to be consistent at all times. If I look at an assignment, I expect a semicolon after it, no matter if it's a variable assignment or a function assignment. Otherwise it's still not consistent.

(I would also argue that the syntax for bound class methods is terrible, because it shouldn't look like an assignment at all, but I cannot help that, they chose so. If it looks like an assignment, then it also looks weird without a semicolon.)

So this issue goes one step further from #1476, because I would like the ability to require semicolons after whatever assignment I do, for consistency.

Update: Without the semicolon in myFunc, IntelliJ IDEA will warn that the statement is unterminated. So it looks to me that TSLint and IDEA disagree.

Additional info

The rule does not complain, if I don't break the line:

private myFunc =  ([param1, param2, param3]) => param1 && && param2 && !param3;

Also, I expect a semicolon here, but there is no error message:

    private myFunc2 = (myParam: boolean): boolean => {
        return true;
    }

[ajafff]

I agree, that it's not consistent to simply ignore bound class methods (only when they span multiple lines).
We could add another option for that. But then they are likely to conflict.
I'm leaning more towards making "ignore-bound-class-methods" ignore that this is a bound class method and lint it like everything else. Technically a breaking change.
Thoughts @adidahiya?

[rkrisztian]

To put it simply: I'm asking for a change that:

• if it looks like an assignment => then semicolon it.
• if it looks like a regular function definition => then don't semicolon it.

Why? Because we're spending too much time on the special cases with not much benefit, we're arguing too much about how to make things more complex, and I don't want to confuse my developers,

[adidahiya]

bottom line: I think banning semicolons on bound class methods by default was a mistake. we should align with prettier and enforce them by default. I agree with @rkrisztian's last comment (#3216 (comment)).

however, this would be a breaking change. so despite the inevitable conflict across rule options, I would like to add this as a new rule option ("strict-bound-class-methods" perhaps) which we can make the default in 6.0.

[rkrisztian]

By the way, the arrow function way of defining bound class methods is actually an assignment by definition (well, I think that ticket can be viewed as such).
Edit: See also the TypeScript 0.9.1 release announcement that adds the semicolon officially.
Thus, it counts as a field assignment, thus I don't really understand why most people don't add the semicolon.

Note: I was also considering that maybe a decorator would handle the problem more nicely, but the TypeScript guys don't want to add to add such (e.g. a @bound annotation), because they don't do anything that can be done with a library. So I was also thinking, either we have proper rule for semicolons, or we ban such usage of arrow functions in favor of a decorator. After all, some people think the arrow function way can be too cryptic, even if just using a lambda function property is so simple compared to using decorators. However, the decorator has the same problem actually: it can also make the code more cryptic...

=> So the current idea of adding strict-bound-class-methods is OK, and thanks.