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 false positive with EmptyLinesCheck. #310

Merged

Conversation

seraku24
Copy link
Contributor

@seraku24 seraku24 commented Nov 7, 2016

The logic incorrectly skipped over lines that had single-line comments filling the entire line. Fixed issue and updated unit tests to prevent regression.

The logic incorrectly skipped over lines that had single-line comments filling the entire line.  Fixed issue and updated unit tests to prevent regression.
@@ -38,7 +38,14 @@ class EmptyLinesCheck extends LineCheckBase {
for (i in 0...checker.lines.length) {
var line = checker.lines[i];
var ranges = getRanges(line);
if (ranges.length == 1 && ranges[0].type != TEXT) continue;
//if (ranges.length == 1 && ranges[0].type != TEXT) continue;
Copy link
Member

Choose a reason for hiding this comment

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

Why leave the old logic in, but commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I'm a dumb dumb.

@codecov-io
Copy link

codecov-io commented Nov 7, 2016

Current coverage is 89.62% (diff: 100%)

Merging #310 into dev will increase coverage by <.01%

@@                dev       #310   diff @@
==========================================
  Files           115        115          
  Lines          7601       7607     +6   
  Methods           0          0          
  Messages          0          0          
  Branches        293        293          
==========================================
+ Hits           6812       6818     +6   
  Misses          529        529          
  Partials        260        260          

Powered by Codecov. Last update 3769783...5ba317c

@Gama11
Copy link
Member

Gama11 commented Nov 7, 2016

That was a quick fix. Looking good in Flixel now, the one EmptyLinesCheck warning left over is actually not a false positive. 👍

@Gama11 Gama11 merged commit 652b3df into HaxeCheckstyle:dev Nov 7, 2016
@seraku24 seraku24 deleted the seraku24-fix-emptylines-false-positive branch November 7, 2016 13:49
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.

3 participants