Indent rule not reporting or fixing indent size violations

[adidahiya]

Bug Report

• TSLint version: 5.3.2
• TypeScript version: 2.3.2
• Running TSLint via: CLI

TypeScript code being linted

export function foo() {
  return 123;
}

with tslint.json configuration:

{
  "extends": ["tslint:latest"],
  "rules": {
    "indent": {
      "options": ["spaces", 4]
    }
  }
}

Actual behavior

No errors with tslint. No fixes with tslint --fix.

Expected behavior

Errors reported with tslint, fixes applied with tslint --fix so that the resulting file looks like:

export function foo() {
    return 123;
}

---

#2723 did not quite work like I expected. The problem seems to be that a failure is only reported if using the wrong whitespace character, not if the indent size is off (like in my example). relevant source.

[glen-84]

@adidahiya See this comment by @nchen63.

[dolanmiu]

From @nchen63:

It fixes tabs -> x spaces and x spaces -> tabs, but doesn't fix x spaces -> y spaces

It SHOULD do x spaces to y spaces though.

[ladjzero]

My project uses both 2 and 4 spaces. Fix rule x spaces to y spaces would be very useful.

[hodavidhara]

I also ran into this issue expecting it to fix size violations, or at least report them as errors so that I can fix them. At the moment it seems the configuration for the indent size documented on the site (https://palantir.github.io/tslint/rules/indent/) doesn't actually do anything.

[rsheptolut]

Yes, please fix this bug. Angular CLI uses 2 spaces for an indentation level when generating a file or a project, and all quotes are single quotes. Then it runs tslint to fix these according to the user's tslint.json. Quotes work well (transforming to double quotes as per my preference), but indentation stays at 2 spaces (while I prefer 4). Tslint only reports an error if it sees an actual TAB character, but seems reasonable that it should also check the number of spaces

[TzviPM]

It seems to me like the implementation being used in https://github.com/palantir/tslint/blob/master/src/rules/indentRule.ts is quite naive, and I wouldn't expect it to work for x spaces -> y spaces. The approach seems to be fine with tabs, based on the comment here, but I can't see this approach as viable for spaces.

Say, for instance, an indentation width of 2 spaces was chosen, and consider this code:

foo = {
    a: {
  b: {
    c: 'c'
  }
    },
    d: 'd'
}

How would our current implementation know to fail this at all? Each sequence of spaces passes the regex / /. Even if we looked for multiples of 2 spaces, this would pass. Similarly, in multi-level indentation scenarios, 4 leading spaces could be a passing scenario, whereas beginning a single level of indentation with 4 spaces should fail.

I think any solution would need to traverse the AST, similar to what eslint does (https://github.com/eslint/eslint/blob/master/lib/rules/indent.js). The downside of that would be that we'd take a slight performance hit for tabs -> spaces or spaces -> tabs. We could get around that by choosing the implementation based on the settings, but I anticipate that the current implementation will fail if a combination of tabs and spaces are used, in which case we should only use the AST-based solution.

I think that at least some of the AST work done in the eslint implementation can be handled by typescript's library directly, so the solution shouldn't be too hard to write up.

[adidahiya]

How would our current implementation know to fail this at all?

It doesn't technically need to. The align rule would prevent you from writing code like that. Sure, we could walk the AST in the indent rule as well. All I care about is that some combination of indent and align allow me to accomplish what I wrote in the issue description.

[TzviPM]

The align rule would prevent you from writing code like that.

Nice! I missed that when I was examining the source code. I'll take a peak over there too. TBH, I kinda like the idea of leveraging existing rules to keep others as naive as possible.

[johnwest80]

I'm experiencing the same issue. The rule below will catch tabs being used instead of spaces, but won't catch an incorrect number of spaces. I can change 2 to any other number, and I still get no errors. I'm using tslint 5.5.0.

"indent": [true, "spaces", 2],

[mDibyo]

Any updates on this? (If someone isn't working on a fix already, I'm willling to give it a go. )

[adidahiya]

@mDibyo go for it

[jscharett]

@mDibyo Have you made any progress on this?

[mDibyo]

I have a partial implementation. Will try to complete it and put up a PR over the weekend

[adidahiya]

Just wanted to give a heads up that this issue has become a lower priority for me since I've started to use prettier formatting with tslint-plugin-prettier for my projects. We might try to add more official support for prettier in the built-in configs in the future and correspondingly adjust focus to core rules that are not formatting-related.

[glen-84]

Prettier, on the other hand, strives to fit the most code into every line.

What if you don't want that?

With the print width set to 120, prettier may produce overly compact, or otherwise undesirable code.

We use 120 characters in our projects.

---

Also, it's yet another dependency. I think that I'd prefer to just use regular TSLint rules.

[adidahiya]

I think that I'd prefer to just use regular TSLint rules.

@glen-84 yeah, that's fine, which is why i'm not saying we should remove all formatting rules from TSLint and delegate completely to an external formatter. Prettier is obviously opinionated and not everyone is going to choose to use it. This issue is still open for PRs.

[qzmfranklin]

This is a functionality that almost everybody uses on a daily basis. Would appreciate if this issue gets more attention.

[hiteshaleriya]

Guys, You can use ter-indent option provided by tslint-eslint-rules.

"ter-indent": [ true, 4, { "SwitchCase": 1 }]

It worked for me. Cheers!

[SpencerKaiser]

@hiteshaleriya that's what I've had in my project for a while now, but it isn't actually fixing the errors, just silencing them... here's my tslint.json:

{
    "extends": "tslint-config-airbnb",
    "rules": {
        "ter-indent": ["error", "spaces", 4],
        "no-unused-vars": ["warn"],
        "no-multi-spaces": false,
        "no-console": false,
        "max-line-length": false,
        "import-name": false
    }
}

and here's a relevant example that finished without causing warnings or errors:

function retrieveAndSetConfig(): Promise<any> {
  return new Promise((resolve, _) => {
  // ^ 2 spaces, expected 4
    const ghe = new GHEUtils();
    // ^ 4 spaces, expected 8
    // ...
}

It's also not showing errors when tabs are used (although that might be by design when 4 spaces are present?).

[hiteshaleriya]

@SpencerKaiser Can you please update your ter-indent rule as shown below and then try:

"ter-indent": [true, 4]

I tried your example and it is working as expected at my end (causing errors).

[SpencerKaiser]

@hiteshaleriya thanks for getting back to me so quickly! So it's now throwing errors as expected (👍) but it's not fixing any of them with --fix. Any ideas?

[hiteshaleriya]

@SpencerKaiser Can you try running the --fix command twice. First time it will indent first line only then second time it will indent the rest (for your example code). Seems weird but please report the issue if it doesn't work.

[SpencerKaiser]

@hiteshaleriya so a couple of observations... I didn't need to run it again, I needed to run it roughly n/4 times where n is the length of the indentation in spaces of the farthest indented line in the project ¯\_(ツ)_/¯

After finally finishing them all, it seems it's skipping over basic indentation errors like this:

class Something {
    function myFunc() {
        const myThing = {
            wat: 1,
         wattt: 5,    // 9 spaces, expected 12
        };
    }
}

If I mess up the indentation level of the const (line 17) to 0 spaces, the rest of it is flagged with errors excluding the line with the space when I leave off --fix:

ERROR: 17:1  ter-indent  Expected indentation of 8 spaces but found 0.
ERROR: 18:1  ter-indent  Expected indentation of 4 spaces but found 12.
ERROR: 20:1  ter-indent  Expected indentation of 0 spaces but found 8.

With --fix, here's the first pass:

        const myThing = {
    wat: 1,
    	 wattt: 5,
};

And the second pass:

        const myThing = {
            wat: 1,
    	 wattt: 5,
        };

Thoughts??

[shubich]

How I've solved that issue with tslint-config-prettier

[SpencerKaiser]

@shubich I ended up doing the same thing...

[MaKCbIMKo]

Is there any update on this?

[cyberhck]

@MaKCbIMKo as far as I understand, the entire team will be moving to integrate eslint visit typescript-eslint, and in near future tslint will be deprecated, so it's as good as ignoring this rule for now (or use the tslint-config-prettier)

[adidahiya]

Closing due to the complexity of this task and the change in project direction: #4534

[linvain]

ESLint rule from typescript-eslint works perfectly for me (see #4534):

module.exports = {
	"env": {
		"browser": true,
	},
	"parser": "@typescript-eslint/parser",
	"parserOptions": {
		"ecmaVersion": 2019,
		"sourceType": "module",
		"ecmaFeatures": {
			"jsx": true
		},
		"project": "./tsconfig.json",
	},
	"plugins": ["@typescript-eslint"],
	"rules": {
		"@typescript-eslint/indent": ["error", 2] // or ["error", "tab"]
	}
}

[thakurvijendar]

How I've solved that issue with tslint-config-prettier

Link not found.

[JoshuaKGoldberg]

🤖 Beep boop! 👉 TSLint is deprecated 👈 and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

[JoshuaKGoldberg]

P.S. tslint-config-prettier - please stop using linters such as TSLint to format your TypeScript. That's better done by a formatter such as Prettier.