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

Improve jsx-closing-bracket-location error message #301

Merged
merged 1 commit into from
Nov 10, 2015

Conversation

alopatin
Copy link
Contributor

@alopatin alopatin commented Nov 9, 2015

By adding expected column information for the closing bracket, as well
as reporting the error at the closing bracket instead of the opening
bracket. Parsing the error message should now be sufficient to allow
writing an external autofixer.

Example input test.jsx with default options:

<Provider
    store> // <--
    <App
        foo
    />
</Provider>

Resulting error:

test.jsx:2:10: Ereact/jsx-closing-bracket-location The closing bracket must be aligned with the opening tag (expected column 1 on the next line)

Note that the reported error location now corresponds to the closing > bracket on line 2, and that the error message now includes the exact location of where that closing bracket should have been.

Test Plan:
npm install
npm run test

Summary:
By adding expected column information for the closing bracket, as well
as reporting the error at the closing bracket instead of the opening
bracket. Parsing the error message should now be sufficient to allow
writing an external autofixer.

Test Plan:
npm install
npm run test

Reviewers: csilvers

Reviewed By: csilvers

Differential Revision: https://phabricator.khanacademy.org/D23050
@yannickcr yannickcr merged commit 218e9fc into jsx-eslint:master Nov 10, 2015
@yannickcr
Copy link
Member

Great! Thanks for this!

Parsing the error message should now be sufficient to allow writing an external autofixer.

Actually since ESLint now have an API to apply fixes we could directly add the autofixer in the rule. There is already an issue for this (#245) but I did not yet had the time to do it.

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

Successfully merging this pull request may close these issues.

2 participants