8000 [indent] Extend base rule to support typescript nodes · Issue #201 · bradzacher/eslint-plugin-typescript · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

[indent] Extend base rule to support typescript nodes #201

Closed
bradzacher opened this issue Dec 4, 2018 · 9 comments
Closed

[indent] Extend base rule to support typescript nodes #201

bradzacher opened this issue Dec 4, 2018 · 9 comments
Labels
8000 new rule/new functionality
Milestone
1.0.0

Comments

@bradzacher
Copy link
Owner
bradzacher commented Dec 4, 2018 •
edited
Loading

#174 adds some tests for indent which covers some default eslint cases.

Unfortunately the rule is designed such that it completely ignores non-standard nodes (i.e. all of the new typescript nodes introduced in parser v20+).

Specifically we need to support:

  • TSTypeLiteral
  • TSInterfaceBody
  • TSImportEqualsDeclaration
@bradzacher bradzacher added the tests anything to do with testing label Dec 4, 2018
@bradzacher bradzacher added this to the 1.0.0 milestone Dec 4, 2018
@armano2
Copy link
Contributor
armano2 commented Dec 4, 2018 •
edited
Loading

@bradzacher tests for this are done in parser, but sure, more test is always better

@bradzacher bradzacher added new rule/new functionality tests anything to do with testing and removed tests anything to do with testing new rule/new functionality labels Dec 5, 2018
@bradzacher
Copy link
Owner Author

You added the test here!
https://github.com/armano2/eslint-plugin-typescript/blob/7cf0b229504e3539fe9bf832cd0352c38234c1c9/tests/lib/rules/indent.js

However, it has valid cases only.
If we add invalid cases we can ensure that decorators/types/interfaces are correctly fixed so we can detect regressions.

@bradzacher
Copy link
Owner Author

Hmm yeah instantly can see the value in this...

I added this invalid test case, and it fails:

        {
            code: `
interface Foo {
bar : string,
age : number,
}
            `,
            output: `
interface Foo {
    bar : string,
    age : number,
}
            `,
            errors: [
                {
                    message: `Expected indentation of 4 spaces but found 0.`,
                    line: 3,
                    column: 1,
                },
                {
                    message: `Expected indentation of 4 spaces but found 0.`,
                    line: 4,
                    column: 1,
                },
            ],
        },

@armano2
Copy link
Contributor
armano2 commented Dec 11, 2018

it means that its working as expected, eslint is not supporting TS nodes, and there is/was bunch of issues that nodes was handled incorrectly.

@armano2
Copy link
Contributor
armano2 commented Dec 11, 2018 •
edited
Loading

we should write new rule that extends eslint rule to support indent in TS specific nodes

@bradzacher
Copy link
Owner Author

As a plugin developer that knows its purposely designed to not support typescript ast nodes, sure it's working correctly.

As a user that wants their interfaces and type literals indented; the rule is broken.

@armano2
Copy link
Contributor
armano2 commented Dec 11, 2018 •
edited
Loading

i can work on this after i solving issue with array-type

@bradzacher
Copy link
Owner Author

I'm already hacking away at it!

@bradzacher bradzacher added new rule/new functionality and removed tests anything to do with testing labels Dec 11, 2018
@bradzacher bradzacher changed the title [indent] Add fixer test cases [indent] Extend base rule to support typescript nodes Dec 11, 2018
@bradzacher bradzacher mentioned this issue Dec 11, 2018
Merged
@ficristo
Copy link

Is it possible to add a fix (if not already fixed) for eslint/typescript-eslint-parser#577 too?

@bradzacher bradzacher mentioned this issue Dec 15, 2018
Closed
bradzacher added a commit that referenced this issue Dec 17, 2018
@bradzacher
[FEAT] [indent] Extend the base rule to support TS nodes (#219)
84808c1 
Fixes #201 
Fixes #96 
Fixes eslint/typescript-eslint-parser#577

The [base eslint implementation](https://github.com/eslint/eslint/blob/master/lib/rules/indent.js) purposely ignores nodes it doesn't know about (i.e. our TS nodes).

Because of how the base rule is written, we have to override the implementation entirely.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
416C No one assigned
Labels
new rule/new functionality
Projects
None yet
Milestone
1.0.0
Development

No branches or pull requests
3 participants
@armano2 @bradzacher @ficristo
0