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 isFirstNested util to handle shared-line comments #2827

Merged
merged 1 commit into from
Sep 2, 2017

Conversation

nwoltman
Copy link
Contributor

Which issue, if any, is this issue related to?

Fixes #2764

Is there anything in the PR that needs further explanation?

This changes the isFirstNested util to ignore comments that are on the same line as the opening brace of a block when checking if the provided node is the first node in the block.

Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

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

LGTM, lots of tests ❤️

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Good works, thanks!

}
}

/* istanbul ignore next: Should always return in the loop */
Copy link
Member

Choose a reason for hiding this comment

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

Why this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's impossible for the code execution to reach here. One of the return statements in the for loop will always be executed. This is guaranteed because it returns if it finds the statement node in parent.nodes and that must happen because that node must be in its own parent's list of nodes.

If this is too strange I could change the return false; in the for loop into break; so then this return false; at the end of the function will be executed.

Copy link
Member

Choose a reason for hiding this comment

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

@nwoltman good, just question, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Why comment starts with “istanbul ignore next:”?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jest uses Istanbul for analyzing code coverage. That comment tells Istanbul not to count that line in the total code coverage since it's impossible to execute (the documentation is in the old Istanbul repo).

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Excellent!

@jeddy3 jeddy3 merged commit cc9ce64 into stylelint:master Sep 2, 2017
@jeddy3
Copy link
Member

jeddy3 commented Sep 2, 2017

Changelog:

  • Fixed: *-empty-line-before false positives shared-line comments and "first-nested" option (#2827).

@nwoltman Thanks for this! Sorry about the delay getting it in. A few of us (the stylelint contributors) have been away/relocating.

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.

False positives for first-nested preceded by a shared-line comment in *-empty-line-before
5 participants