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

added ignoreEmptyLines to FileLength and MethodLength checks #486

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ after_script:
- if [[ "$HAXE_VERSION" == "haxe4" ]]; then (cd src; ../cc-test-reporter upload-coverage); fi

after_success:
- bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
- if [[ "$HAXE_VERSION" == "haxe4" ]]; bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

## dev branch / next version (2.x.x)

- New check `CodeSimilarity` to check for similar or identical code blocks ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479) + [#480](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/480) + [#484](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/484))
- **Breaking Change** changed `MethodLength.countEmpty` into `ignoreEmptyLines`
- New check `CodeSimilarity` to check for similar or identical code blocks ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479) + [#480](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/480) + [#484](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/484) + [#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
- New check `EnforceVarTypeHint` to enforce type hints for all variables and finals, fixes [#464](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/464) ([#481](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/481) + [#482](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/482))
- New check `AvoidIdentifier` marks identifiers to avoid ([#483](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/483))
- New check `ArrowFunction` to check for curlies, nested functions and returns in arrow functions ([#484](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/484))
- New check `NestedControlFlow` to check for nested control flow expressions (e.g. `if`, `for`, `while`, `do/while`, `switch` and `try`) ([#485](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/485))
- Added coverage upload to codeclimate ([#478](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/478))
- Added `ignoreEmptyLines` in FileLengthCheck to ignore empty lines (default = true) ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
- Changed default value for `max` in `FileLengthCheck` to 1000 ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
- Changed `MethodLength` check to use tokentree ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
- Fixed allow excluding construtor (`new`) via range exclusion ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479))
- Refactored build system to use lix ([#478](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/478))
- Refactored / renamed AvoidInlineConditionals to AvoidTernaryOperator ([#479](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/479))
Expand Down
3 changes: 0 additions & 3 deletions checkstyle.json
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,6 @@
}
},
{
"props": {
"max": 2000
},
"type": "FileLength"
},
{
Expand Down
19 changes: 12 additions & 7 deletions resources/checkstyle-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,11 @@
"ERROR",
"IGNORE"
],
"propertyOrder": 2
},
"ignoreEmptyLines": {
"description": "ignores or includes empty lines when counting total file length",
"type": "boolean",
"propertyOrder": 1
}
},
Expand Down Expand Up @@ -3513,12 +3518,12 @@
"additionalProperties": false,
"properties": {
"thresholdSimilar": {
"description": "threshold for similar code blocks",
"description": "maximum number of tokens allowed before detecting similar code blocks",
"type": "integer",
"propertyOrder": 2
},
"thresholdIdentical": {
"description": "threshold for identical code blocks",
"description": "maximum number of tokens allowed before detecting identical code blocks",
"type": "integer",
"propertyOrder": 1
},
Expand Down Expand Up @@ -3927,11 +3932,6 @@
"description": "Checks for long methods. If a method becomes very long it is hard to understand. Therefore long methods should usually be refactored into several individual methods that focus on a specific task.",
"additionalProperties": false,
"properties": {
"countEmpty": {
"description": "maximum includes empty lines / should ignore empty lines",
"type": "boolean",
"propertyOrder": 1
},
"max": {
"description": "maximum number of lines per method (default: 50)",
"type": "integer",
Expand All @@ -3947,6 +3947,11 @@
"IGNORE"
],
"propertyOrder": 2
},
"ignoreEmptyLines": {
"description": "ignores or includes empty lines when counting method length",
"type": "boolean",
"propertyOrder": 1
}
},
"type": "object"
Expand Down
9 changes: 5 additions & 4 deletions resources/default-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
"type": "CodeSimilarity",
"props": {
"severityIdentical": "WARNING",
"thresholdIdentical": 8,
"thresholdSimilar": 12
"thresholdIdentical": 60,
"thresholdSimilar": 120
}
},
{
Expand Down Expand Up @@ -192,7 +192,8 @@
{
"type": "FileLength",
"props": {
"max": 2000
"max": 1000,
"ignoreEmptyLines": true
}
},
{
Expand Down Expand Up @@ -323,7 +324,7 @@
"type": "MethodLength",
"props": {
"max": 50,
"countEmpty": false
"ignoreEmptyLines": true
}
},
{
Expand Down
41 changes: 24 additions & 17 deletions src/checkstyle/checks/coding/CodeSimilarityCheck.hx
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,20 @@ class CodeSimilarityCheck extends Check {
public var severityIdentical:SeverityLevel;

/**
threshold for identical code blocks
maximum number of tokens allowed before detecting identical code blocks
**/
public var thresholdIdentical:Int;

/**
threshold for similar code blocks
maximum number of tokens allowed before detecting similar code blocks
**/
public var thresholdSimilar:Int;

public function new() {
super(TOKEN);
severityIdentical = WARNING;
thresholdIdentical = 8;
thresholdSimilar = 12;
thresholdIdentical = 60;
thresholdSimilar = 120;
categories = [STYLE, DUPLICATION];
}

Expand Down Expand Up @@ -82,10 +82,9 @@ class CodeSimilarityCheck extends Check {

var lineStart:LinePos = checker.getLinePos(pos.min);
var lineEnd:LinePos = checker.getLinePos(pos.max);
var lines:Int = lineEnd.line - lineStart.line;
if (lines <= Math.min(thresholdIdentical, thresholdSimilar)) return false;

var hashes:CodeHashes = makeCodeHashes(token);
if (hashes.tokenCount <= Math.min(thresholdIdentical, thresholdSimilar)) return false;
var codeBlock:HashedCodeBlock = {
fileName: token.pos.file,
lineStart: lineStart,
Expand All @@ -94,15 +93,15 @@ class CodeSimilarityCheck extends Check {
endColumn: offsetToColumn(lineEnd)
}

if (lines > thresholdIdentical) {
if (hashes.tokenCount > thresholdIdentical) {
var existing:Null<HashedCodeBlock> = checkOrAddHash(hashes.identicalHash, codeBlock, IDENTICAL_HASHES);
if (existing != null) {
logRange("Found identical code block - " + formatFirstFound(existing), pos.min, pos.max, SIMILAR_BLOCK, ERROR);
logRange("Found identical code block - " + formatFirstFound(existing), pos.min, pos.max, SIMILAR_BLOCK, severityIdentical);
return true;
}
}

if (lines > thresholdSimilar) {
if (hashes.tokenCount > thresholdSimilar) {
var existing:Null<HashedCodeBlock> = checkOrAddHash(hashes.similarHash, codeBlock, SIMILAR_HASHES);
if (existing == null) return false;
logRange("Found similar code block - " + formatFirstFound(existing), pos.min, pos.max, SIMILAR_BLOCK);
Expand All @@ -126,19 +125,26 @@ class CodeSimilarityCheck extends Check {
function makeCodeHashes(token:TokenTree):CodeHashes {
var similar:StringBuf = new StringBuf();
var identical:StringBuf = new StringBuf();
makeCodeHashesRecursive(token, similar, identical);
var tokenCount:Int = makeCodeHashesRecursive(token, similar, identical);
return {
identicalHash: identical.toString(),
similarHash: similar.toString()
similarHash: similar.toString(),
tokenCount: tokenCount
};
}

function makeCodeHashesRecursive(token:TokenTree, similar:StringBuf, identical:StringBuf) {
function makeCodeHashesRecursive(token:TokenTree, similar:StringBuf, identical:StringBuf):Int {
similar.add(similarTokenText(token));
identical.add(identicalTokenText(token));
var count:Int = 0;
var identicalText:Null<String> = identicalTokenText(token);
if (identicalText != null) {
count++;
identical.add(identicalText);
}
if (token.children != null) {
for (child in token.children) makeCodeHashesRecursive(child, similar, identical);
for (child in token.children) count += makeCodeHashesRecursive(child, similar, identical);
}
return count;
}

function similarTokenText(token:TokenTree):String {
Expand Down Expand Up @@ -184,7 +190,7 @@ class CodeSimilarityCheck extends Check {
}
}

function identicalTokenText(token:TokenTree):String {
function identicalTokenText(token:TokenTree):Null<String> {
switch (token.tok) {
case Const(CFloat(f)):
return '$f';
Expand All @@ -203,9 +209,9 @@ class CodeSimilarityCheck extends Check {
case Binop(op):
return '$op';
case Comment(_):
return "";
return null;
case CommentLine(_):
return "";
return null;
case IntInterval(i):
return '...$i';
default:
Expand Down Expand Up @@ -241,4 +247,5 @@ typedef HashedCodeBlock = {
typedef CodeHashes = {
var identicalHash:String;
var similarHash:String;
var tokenCount:Int;
}
21 changes: 18 additions & 3 deletions src/checkstyle/checks/size/FileLengthCheck.hx
Original file line number Diff line number Diff line change
@@ -1,36 +1,51 @@
package checkstyle.checks.size;

import checkstyle.checks.whitespace.ListOfEmptyLines;

/**
Checks for long source files. If a source file becomes very long it is hard to understand.
Therefore long classes should usually be refactored into several individual classes that focus on a specific task.
**/
@name("FileLength")
@desc("Checks for long source files. If a source file becomes very long it is hard to understand. Therefore long classes should usually be refactored into several individual classes that focus on a specific task.")
class FileLengthCheck extends Check {
static var DEFAULT_MAX_LENGTH:Int = 2000;
static var DEFAULT_MAX_LENGTH:Int = 1000;

/**
maximum number of lines permitted per file (default: 2000)
**/
public var max:Int;

/**
ignores or includes empty lines when counting total file length
**/
public var ignoreEmptyLines:Bool;

public function new() {
super(LINE);
max = DEFAULT_MAX_LENGTH;
ignoreEmptyLines = true;
categories = [Category.COMPLEXITY, Category.CLARITY];
points = 21;
}

override function actualRun() {
if (checker.ast == null) return;

for (td in checker.ast.decls) {
switch (td.decl) {
case EClass(d):
for (field in d.data) if (isCheckSuppressed(field)) return;
default:
}
}
if (checker.lines.length > max) {

var count = checker.lines.length;
if (ignoreEmptyLines) {
var emptyLines:ListOfEmptyLines = ListOfEmptyLines.detectEmptyLines(checker);
count -= emptyLines.lines.length;
}
if (count > max) {
log('File length is ${checker.lines.length} lines (max allowed is ${max})', checker.lines.length, 0, checker.lines.length, 0);
}
}
Expand All @@ -40,7 +55,7 @@ class FileLengthCheck extends Check {
fixed: [],
properties: [{
propertyName: "max",
values: [for (i in 0...10) 400 + i * 200]
values: [for (i in 0...10) 400 + i * 100]
}]
}];
}
Expand Down
Loading