-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[indent] incorr 8000 ect treatment of multi-line parameter types #880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I'm having this same problem - I'd like arrow function indents to be treated the same as function expressions. What's particularly bad about this case is that you can't even disable the rule by turning off indent validation for function expressions; it means that you are basically forced into mis-indenting your code in order to get it to pass the linter. |
@bradzacher let me know if there's anything I can help w/ for the rewrite - I've just finished rewriting |
It's a pretty huge job, which is why I haven't worked on it. The current rule implementation does some super duper dodgy stuff in the form of converting TSESTree AST nodes into "similar looking" standard ESTree AST nodes. The problem with this is that not everything has a 1:1 representation, and some things look different. For example - unions/intersections get converted to binary expressions. This kinda works, except for some cases where the syntax doesn't match up, causing the algorithm to go awry. This problem compounds a bit when you've got typescript specific nodes within typescript specific nodes. As you've then got hacked together indentation within hacked together indentation... It's messy. On top of that, the base eslint rule is actually super relaxed, and doesn't check things that you'd expect. For example - a roughly equivalent JS version of your example would be: const j = (
n = string,
voo =
string
| Record
| DependencyLock
) => {
const version = typeof voo !== 'string'
? voo[n]
: voo;
return `${n}@${version}`;
}; Note the weird indentation I've put in? That's because eslint's indent rule doesn't actually validate the indentation of binary expressions, so that's valid according to the rule... So what does this mean for the indent rule? So I decided that rather than calling eslint's implementation, it'd be better to just reimplement eslint's implementation, and then add true support for TS nodes (and probably fix up the weird, weird relaxed cases). Step one of that was copy-pasting their code into this repo, and then converting it to typescript. Step two is to start adding support for TS nodes. I haven't had the motivation to sit down for the (what will probably be) 10+ hours to understand the algorithm, add in all of the typescript nodes, add tests, etc. Mainly because I don't use the rule any more - I just use prettier everywhere. If you want to take this on, I'd be ecstatic! It's on my todo list for the next time I'm stuck on a plane with nowhere else to go or do (or sometime like that). |
Hmmm yes that is an algorithm alright 😂 I'll have a deeper look & maybe a play when I've got some spare time, but won't make promises at this point due to size & other commitments. This might be obvious after a deeper look (or more general eslint/AST exp.), but currently the big question on my mind is:
What does that actually mean? If I'm understanding you correctly, you've already converted the indent rule to TS, so ideally isn't it now a matter of implementing all the different TS options people want (as they come in)? I suspect I've missed something, but if it is that, then I should able to semi-commit to incrementally tackle adding support for options as they come in 🙂 |
Sounds simple right? 😃 Some of it will probably be simple. An interface/object type/enum can follow similar logic to objects, union/intersections follow similar logic to binary expressions, etc, etc. There's a bunch of prior art you can work from within the rule. The problem is that it can be really ugly handling all of the cases properly for a single node. type T = foo | bar
// ^^^^^^^^^ expected indent = 0
type T = foo
// ^^^ expected indent = 0
| bar
// ^^^^^ expected indent = 1
type T =
| foo
// ^^^^^ expected indent = 1
| bar
// ^^^^^ expected indent = 1 The logic can be non-trivial 😅 for some nodes. |
Is there a way to ignore only the methods/functions part of indentation? so to suppress those kind of warnings without disabling the entirety of |
I'm hitting a similar problem: export type TypedArray = Uint8Array | Uint16Array | Uint32Array | Uint8ClampedArray
| Int8Array | Int16Array | Int32Array
| Float32Array | Float64Array; Errors on 2 last lines:
|
Merging into #1824 |
Uh oh!
There was an error while loading. Please reload this page.
Just to throw my hat into the "rewrite indent" ring:
Repro
https://github.com/G-Rath/js-bugs/tree/typescript-eslint/issue-880
Expected Result
No error.
Actual Result
ESLint: Expected indentation of 2 spaces but found 4 @typescript-eslint/indent
Additional Info
There were a couple of issues that looked very related, but none that quite hit the nail on the head; sorry if this is a duplicate 😬
Versions
@typescript-eslint/eslint-plugin
2.0.0
@typescript-eslint/parser
2.0.0
TypeScript
3.5.3
ESLint
6.2.0
node
10.15.3
npm
6.10.3
The text was updated successfully, but these errors were encountered: