eslint / typescript-eslint-parser

An ESLint custom parser which leverages TypeScript ESTree to allow for ESLint to lint TypeScript source code.
Other
915 stars 92 forks source link

Indentation warning for properties of argument object #458

Closed bondom closed 5 years ago

bondom commented 6 years ago

What version of TypeScript are you using? 2.7.1

What version of typescript-eslint-parser are you using? 14.0.0

What code were you trying to parse?

const registration = (
  state: RegistrationStoreState = initialState,
  action: {
    type: string;
    data: Object;
    error: Object;
  }
) => {
...
}

What did you expect to happen? To see clear worskpace without warning

What happened?

I see warning [eslint] Expected indentation of 2 spaces but found 4. (indent) And only when I format this code in this way:


const registration = (
  state: RegistrationStoreState = initialState,
  action: {
  type: string;
  data: Object;
  error: Object;
  }
) => {
...
}

I don't see warnings.

mightyiam commented 6 years ago

Something is wrong with your code. It looks like mixed JavaScript primitives and TypeScript types.

bondom commented 6 years ago

@mightyiam Sorry, but I don't understand. What do you mean under Javaccript primitives? I am newbie in TypeScript, but there is how I understand this: Javascript primitives are the same as Typescript primitives. In my code above I have string primitive, which is valid primitive type in Typescript. But I have Object, that is also TypeScript type. For experiment I replaced Object with object(which is also valid Typescript type). Now my code is:

const registration = (
  state: RegistrationStoreState = initialState,
  action: {
    type: string;
    data: object;
    error: object;
  }
) => {
...
}

But warning still presents..

bondom commented 6 years ago

I created piece of code with function instead of arrow function:

function reg(
  firstArg: string,
  secondArg: {
    type: string;
    data: object;
    error: object;
  }
): number {
  return 4;
}

But warning also is shown... If this piece of code is mixed Javascript and Typescript, please rewrite it to be only Typescript

bondom commented 6 years ago

Ok, so I solved this issue in this way: 1) Deleted indent rule("indent": ["warn", 2, { "SwitchCase": 1 }]) from eslint config(as a result no warning is shown) 2) Added

"prettier/prettier": [
      "error",
       {
          "parser": "typescript"
      }
]

to my eslint config, so now prettier solves all indentation problems.

Now all works as expected. This code is valid:

const registration = (
  state: RegistrationStoreState = initialState,
  action: {
    type: string;
    data: Object;
    error: Object;
  }
) => {
...
}

But it is obvious that there is real bug with indent rule if we use eslint and typescript-eslint-parser together.(if not please show me similar piece of code without this indentation warning).

Because of prettier and typescript-eslint-parser work in right way, but eslint and typescript-eslint-parser don't, I suppose that this bug is related to eslint but not to typescript-eslint-parser @mightyiam How do you think?

mightyiam commented 6 years ago

Sorry, I was wrong. Yeah, it seems like we have an indentation bug, perhaps. Perhaps you could submit a PR that includes a failing test case? You could try to fix, if you like, but a failing test case would be a step towards, as well.

bondom commented 6 years ago

@mightyiam Okay, I will create pr, and will try to fix :)

bondom commented 6 years ago

First I should to know exactly where is cause of problem. If it is eslint problem I should create PR in eslint repo, if it is typescript-eslint-parser I should create PR there. Right?

bondom commented 6 years ago

Can you please give advice where should I start from? Should I understand main part of eslint source code to be able to understand the cause of issue?

mightyiam commented 6 years ago

I would start by adding a test file here that is based on this. The main difference would be that this parser is used. And add a test that would fail, as well.

Kimahriman commented 6 years ago

If you're using ESLint 4.19+, it may be related to a bug I posted in ESLint around the same time as this.

https://github.com/eslint/eslint/issues/10117

I found the exact commit and changes that caused the issue on the ESLint side. I wasn't sure if that change was wrong or it just required some update here, but it might help figure out the issue.

JamesHenry commented 5 years ago

This issue has been migrated to the new project: typescript-eslint/typescript-eslint#11

Thanks!