Skip to content
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

Fix: Support ESLint <3.11.0 #24

Merged
merged 1 commit into from
May 19, 2017
Merged

Fix: Support ESLint <3.11.0 #24

merged 1 commit into from
May 19, 2017

Conversation

zertosh
Copy link
Member

@zertosh zertosh commented May 18, 2017

Converting from a range/index to a location varies between ESLint versions. This PR covers versions all the way back to ESLint 3.9.0, and creates a test matrix of the various versions.

Context: Unfortunately, we have a project with ESLint 3.9.0 that I can't update right now without making a mess.

};
}
// Add 1 for the line ending character
offset = offset + lines[i].length + 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be incorrect if a file contains CRLF newlines, because the line ending will have a length of 2.

IIRC, there isn't enough information in the lines array to compute this accurately because it doesn't store which linebreaks were used. astUtils.getLocationFromRangeIndex was implemented by splitting the text into lines again and including linebreaks in the result.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so true - can you tell I don't interact with windows much? 😛 I circle back to this tmw.

const sourceCode = context.getSourceCode();
if (typeof sourceCode.getLocFromIndex === 'function') {
return sourceCode.getLocFromIndex(index);
}
// Otherwise, try to use the private version from eslint/lib/ast-utils -
// added in ESLint 3.11.0.
const astUtils = require('eslint/lib/ast-utils');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't figured out the root cause yet, but I think using require('eslint') might be the source of #11 and #19. That could happen if the module found by running require('eslint') is not the same as the eslint module that is actually running the rule, which could result in neither sourceCode.getLocFromIndex nor astUtils.getLocationFromRangeIndex existing even when both modules are above 3.11.0.

That said, I guess this isn't a problem anymore because the fallback will get used now if neither function exists.

@zertosh
Copy link
Member Author

zertosh commented May 18, 2017

@not-an-aardvark, I removed the use of eslint/lib/ast-utils, and went with a CRLF-friendly fallback implementation of getLocFromIndex when it's not available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants