Tab indentation breaks multiple rules #3151

Arcanemagus · 2017-06-23T19:20:52Z

It seems that indentation using tabs breaks the position reported by at least 3 rules, likely more.

All of the following was linted using this .jshintrc file:

{
  "strict": "global",
  "unused": true
}

The JSHint version tested is v2.9.5.

If you want me to refile these as separate issues I can do that as well.

W032

If you lint this code:

'use strict';

	function foo() {
	};

foo();

There is an error reported on line 4, col 6, but line 4 only has 4 characters in it.

W098

If you lint this code:

'use strict';

	function foobar(
		$foo
	) {
		return 'foo';
	}

foobar();

There is an error reported on line 4, col 9, but line 4 only has 7 characters in it.

W117

If you lint this code:

'use strict';

	function foobar() {
		if (true) {
			fun1();
		}
	}

foobar();

There is an error reported on line 5, col 13, but line 4 only has 11 characters in it.

The character position reported seems to go further and further out the more tabs there are at the beginning of the line in question.

Raw files can be found here: linter-jshint_GH416.zip

Originally discovered while investigating AtomLinter/linter-jshint#416.

Rules known to be affected:

W009
W014
E015
W024
W027
W030
W032
W033
W040
W043
W069
W075
W098
W116
W119
W140
E041
Many others...

The text was updated successfully, but these errors were encountered:

jugglinmike · 2017-06-25T18:04:51Z

Thanks for the report! The problem stems from line 1610 of lex.js:

this.input = this.input.replace(/\t/g, state.tab);

This seems pretty fundamental to how many of the style-related linting rules are implemented. Those have been deprecated for some time now, but we're still not ready to move on a new major version. So in the mean time, we'll need to design a solution that preserves that behavior.

My initial thought is to track this "effective offset" as a new property of the token object named column. This can be used for those style-related warnings. Then, the character attribute could be re-implemented to describe the "true" character offset, where each code point (tab or otherwise) increments the count by exactly 1. This value would be used to issue warnings. Reporter plugins would then be in control of how they rendered the tab character, and they could interpret the reported "column" number accordingly.

Does that sound good to you, @Arcanemagus?

Arcanemagus · 2017-06-25T20:28:21Z

That sounds perfect to me, at least on the surface with no real knowledge of JSHint's internals 😛. Ideally I just need some method of getting the true column count (where each code point counts as one column).

This was hidden for a while due to this warning essentially being ignored due to a workaround for some old parser bug with a semi-common error. When I re-implemented the warning to users in a much nicer manner I had accidentally hidden them, and just recently fixed it making this visible again.

Arcanemagus · 2017-06-26T15:51:03Z

Looks like W030, W033, and W009 are also affected. From your description I'm assuming all rules are affected?

justinAurand · 2017-06-29T21:17:11Z

I'm also experiencing this issue and am able to replicate on both errors and warnings. Rule/Error type doesn't seem to matter.

Here's an example of jshint attempting to warn me my '==' should be '==='. Note the erroneous location of the orange underline:

Hopefully this confirms your thoughts on the issue.

jugglinmike · 2017-07-01T20:43:54Z

I got some time to do some digging today, and I believe setting the indent option to 1 will produce the desired results.

@Arcanemagus Are you able to over-ride the value of the indent option for your consumers? If this is possible, I'd rather do that, since I'm not sure how changing the default value in JSHint itself could effect its consumers.

Arcanemagus · 2017-07-02T19:45:39Z

Hmmm, currently this project uses the CLI interface it looks like. It's possible to rewrite it to use the Node.js API which would make that possible.

Would forcing the indent value change the results consumers see? The goal of that package is to be as close to possible as it can be of running jshint themselves, but getting the results integrated in the editor.
Does the JSHint API have a hidden method to grab the configuration for a file, or would that need to be implemented? The current documentation doesn't show such a function.

cobexer · 2017-07-05T06:45:38Z

How about adding a command line argument that allows to override values from the configuration?
Something like that is common in tools like -g in nginx, -o in ssh, -c in git and many more I guess.

$ jshint -o "indent = 1" -o "-W034 = true" -o "globals.require = false" -o "globals.$ = null"

jaredatch · 2017-07-19T16:11:42Z

Does anyone know what version this was introduced in? 2.9.5?

At least then we could downgrade for the moment as a temp. fix.

Arcanemagus · 2017-07-19T16:20:13Z

@jaredatch Back in AtomLinter/linter-jshint#386 (released in v3.1.0 of linter-jshint) all of the workarounds for JSHint's buggy point reporting were removed. The entire point of that check is to find bugs like this were the linter is reporting invalid points so it can get reported and fixed for everyone using the linter. (After all, if you can't trust the linter to give you accurate data, why use it?)

There may be a "sweet spot" before this tab bug was introduced and after the major parser bugs that caused those workarounds to be put in place in the first place.

jaredatch · 2017-07-19T16:28:58Z

@Arcanemagus gotcha, really appreciate the insight. Keep up the great work 👍

Arcanemagus · 2017-07-19T16:31:33Z

If you have a better / more useful message idea for how linter-jshint reports these invalid points feel free to file an issue / submit a PR over there 😉.

yeldarby · 2018-01-03T18:56:17Z

Any update on this issue?

Arcanemagus · 2018-03-16T20:35:26Z

Looks like we're at at least 40 different rules affected by this bug now 😛.

xcrap · 2018-03-25T00:47:17Z

Any updates on this, it's some months since I'm having this errors, driving me nuts :)

jugglinmike · 2018-03-25T16:51:57Z

Hi @xcrap! I just checked the comment thread, and it doesn't look like anyone has posted any updates. The good news is that JSHint is an open source project, so you can help fix the problem if it's causing you stress! Would you like to lend a hand? I'd be happy to advise you.

xcrap · 2018-03-25T18:28:34Z

@jugglinmike I wish! I always try in tiny projects but my js skills are super basic and limited :)

yeldarby · 2018-04-25T16:11:27Z

I placed a $50 bounty on this bug, others can contribute to increase the bounty via this link: https://www.bountysource.com/issues/46533252-tab-indentation-breaks-multiple-rules

By tracking the column number both with and without soft tabs, we enable better messages for errors and warnings. This is especially useful for editor linting integrations. Fixed jshint#3151

yeldarby · 2020-11-07T15:17:49Z

Pulling in @TzviPM's fork locally and rebuilding jshint worked to fix the issue for me if anyone else is stumbling across this:

git clone https://github.com/tzvipm/jshint
git remote add upstream git@github.com:jshint/jshint.git
git fetch upstream
git stash
git merge upstream/master
git mergetool --tool=opendiff
npm run build

jugglinmike added the Needs Discussion label Jun 25, 2017

This was referenced Jun 26, 2017

Invalid position given by 'W030' AtomLinter/linter-jshint#421

Closed

Invalid position given by 'W033' AtomLinter/linter-jshint#422

Open

Arcanemagus mentioned this issue Jun 30, 2017

Invalid position given by 'W032' AtomLinter/linter-jshint#444

Open

This was referenced Jul 2, 2017

Invalid position given by 'W119' AtomLinter/linter-jshint#442

Open

Invalid position given by 'W140' AtomLinter/linter-jshint#448

Open

This was referenced Jul 17, 2017

Invalid position given by 'W116' AtomLinter/linter-jshint#432

Open

Invalid position given by 'W027' AtomLinter/linter-jshint#458

Open

Arcanemagus mentioned this issue Oct 2, 2017

Invalid position given by 'E058' AtomLinter/linter-jshint#518

Open

This was referenced Oct 10, 2017

Invalid position given by 'W100' AtomLinter/linter-jshint#522

Open

Invalid position given by 'W106' AtomLinter/linter-jshint#523

Open

Invalid position given by 'W010' AtomLinter/linter-jshint#524

Open

This was referenced Oct 19, 2017

Invalid position given by 'W108' AtomLinter/linter-jshint#527

Closed

Invalid position given by 'W101' AtomLinter/linter-jshint#530

Closed

Invalid position given by 'W004' AtomLinter/linter-jshint#532

Open

Arcanemagus mentioned this issue Dec 22, 2017

Invalid position given by 'E054' AtomLinter/linter-jshint#545

Open

Arcanemagus mentioned this issue Jan 4, 2018

Invalid position given by 'W058' AtomLinter/linter-jshint#551

Open

RMacfarlane mentioned this issue Mar 6, 2018

Highlight is not correct when using tabs as indent microsoft/vscode-jshint#41

Open

This was referenced Mar 16, 2018

Invalid position given by 'E043' AtomLinter/linter-jshint#574

Open

Invalid position given by 'W107' AtomLinter/linter-jshint#572

Open

Arcanemagus mentioned this issue Mar 29, 2018

Invalid position given by 'W110' AtomLinter/linter-jshint#577

Open

Arcanemagus mentioned this issue Apr 5, 2018

Invalid position given by 'E003' AtomLinter/linter-jshint#580

Open

TzviPM linked a pull request Apr 26, 2018 that will close this issue

[[FIX]] Improve handling of tab indentation. #3274

Open

Arcanemagus mentioned this issue May 8, 2018

Invalid position given by 'W112' AtomLinter/linter-jshint#585

Open

Arcanemagus mentioned this issue Jun 7, 2018

Invalid position given by 'W087' AtomLinter/linter-jshint#588

Open

Arcanemagus mentioned this issue Jan 20, 2019

Invalid position given by 'E020' AtomLinter/linter-jshint#676

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tab indentation breaks multiple rules #3151

Tab indentation breaks multiple rules #3151

Arcanemagus commented Jun 23, 2017 •

edited

jugglinmike commented Jun 25, 2017

Arcanemagus commented Jun 25, 2017 •

edited

Arcanemagus commented Jun 26, 2017 •

edited

justinAurand commented Jun 29, 2017

jugglinmike commented Jul 1, 2017

Arcanemagus commented Jul 2, 2017

cobexer commented Jul 5, 2017

jaredatch commented Jul 19, 2017

Arcanemagus commented Jul 19, 2017

jaredatch commented Jul 19, 2017

Arcanemagus commented Jul 19, 2017

yeldarby commented Jan 3, 2018

Arcanemagus commented Mar 16, 2018

xcrap commented Mar 25, 2018

jugglinmike commented Mar 25, 2018

xcrap commented Mar 25, 2018

yeldarby commented Apr 25, 2018

yeldarby commented Nov 7, 2020

Tab indentation breaks multiple rules #3151

Tab indentation breaks multiple rules #3151

Comments

Arcanemagus commented Jun 23, 2017 • edited

W032

W098

W117

jugglinmike commented Jun 25, 2017

Arcanemagus commented Jun 25, 2017 • edited

Arcanemagus commented Jun 26, 2017 • edited

justinAurand commented Jun 29, 2017

jugglinmike commented Jul 1, 2017

Arcanemagus commented Jul 2, 2017

cobexer commented Jul 5, 2017

jaredatch commented Jul 19, 2017

Arcanemagus commented Jul 19, 2017

jaredatch commented Jul 19, 2017

Arcanemagus commented Jul 19, 2017

yeldarby commented Jan 3, 2018

Arcanemagus commented Mar 16, 2018

xcrap commented Mar 25, 2018

jugglinmike commented Mar 25, 2018

xcrap commented Mar 25, 2018

yeldarby commented Apr 25, 2018

yeldarby commented Nov 7, 2020

Arcanemagus commented Jun 23, 2017 •

edited

Arcanemagus commented Jun 25, 2017 •

edited

Arcanemagus commented Jun 26, 2017 •

edited