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 allowFinal to VariableInitialisation, fixes #491 #492

Merged
merged 3 commits into from
Dec 14, 2019
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

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

- Added `allowFinal` setting to `VariableInitialisation`, fixes [#491](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/491) ([#492](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/492))

## version 2.6.0 (2019-12-01)

- **Breaking Change** changed `MethodLength.countEmpty` into `ignoreEmptyLines` ([#486](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/486))
Expand Down
7 changes: 6 additions & 1 deletion resources/checkstyle-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -3900,6 +3900,11 @@
"description": "Checks for instance variables that are initialised at class level.",
"additionalProperties": false,
"properties": {
"allowFinal": {
"description": "final fields must be initialised either immediately or in constructor\n\t\twhen allowFinal is true then VariableInitialisation won't complain about initialisation at class level for final fields",
"type": "boolean",
"propertyOrder": 0
},
"severity": {
"description": "sets gravity of reported violations:\n\t- IGNORE = do not report violations, violations do not appear anywhere in output\n\t- INFO = all violations have info / lowest priority\n\t- WARNING = all violations have warning / medium priority\n\t- ERROR = all violations have error / highest priority",
"type": "string",
Expand All @@ -3909,7 +3914,7 @@
"ERROR",
"IGNORE"
],
"propertyOrder": 0
"propertyOrder": 1
}
},
"type": "object"
Expand Down
4 changes: 3 additions & 1 deletion resources/default-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,9 @@
},
{
"type": "VariableInitialisation",
"props": {}
"props": {
"allowFinal": false
}
},
{
"type": "WhitespaceAfter",
Expand Down
25 changes: 19 additions & 6 deletions src/checkstyle/checks/coding/VariableInitialisationCheck.hx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@ package checkstyle.checks.coding;
@name("VariableInitialisation")
@desc("Checks for instance variables that are initialised at class level.")
class VariableInitialisationCheck extends Check {
/**
final fields must be initialised either immediately or in constructor
when allowFinal is true then VariableInitialisation won't complain about initialisation at class level for final fields
**/
public var allowFinal:Bool;

public function new() {
super(AST);
categories = [Category.STYLE, Category.CLARITY];
points = 2;
allowFinal = false;
}

override function actualRun() {
Expand All @@ -21,14 +28,20 @@ class VariableInitialisationCheck extends Check {

var isPrivate = false;
var isPublic = false;
var isInline = false;
var isStatic = false;
var isInline = f.access.contains(AInline);
var isStatic = f.access.contains(AStatic);
var isFinal = false;

if (isInline || isStatic) return;

if (f.access.contains(AInline)) isInline = true;
else if (f.access.contains(AStatic)) isStatic = true;
else if (f.access.contains(APublic)) isPublic = true;
if (f.access.contains(APublic)) isPublic = true;
else isPrivate = true;

#if haxe4
if (f.access.contains(AFinal)) isFinal = true;
#end
if (allowFinal && isFinal) return;

if (isPrivate || isPublic) {
switch (f.kind) {
case FVar(t, e):
Expand All @@ -40,6 +53,6 @@ class VariableInitialisationCheck extends Check {
}

function warnVarInit(name:String, pos:Position) {
logPos('Invalid variable "${name}" initialisation (move initialisation to constructor or function)', pos);
logPos('Invalid variable initialisation for "${name}" (move initialisation to constructor or function)', pos);
}
}
27 changes: 26 additions & 1 deletion test/checkstyle/checks/coding/VariableInitialisationCheckTest.hx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package checkstyle.checks.coding;
class VariableInitialisationCheckTest extends CheckTestCase<VariableInitialisationCheckTests> {
@Test
public function testVar() {
assertMsg(new VariableInitialisationCheck(), TEST1, 'Invalid variable "_a" initialisation (move initialisation to constructor or function)');
assertMsg(new VariableInitialisationCheck(), TEST1, 'Invalid variable initialisation for "_a" (move initialisation to constructor or function)');
}

@Test
Expand All @@ -15,6 +15,18 @@ class VariableInitialisationCheckTest extends CheckTestCase<VariableInitialisati
public function testEnumAbstract() {
assertNoMsg(new VariableInitialisationCheck(), TEST3);
}

#if haxe4
@Test
public function testFinal() {
var check:VariableInitialisationCheck = new VariableInitialisationCheck();
assertMsg(check, FINAL, 'Invalid variable initialisation for "VALUE" (move initialisation to constructor or function)');
assertNoMsg(check, FINAL_CONSTRUCTOR);
check.allowFinal = true;
assertNoMsg(check, FINAL);
assertNoMsg(check, FINAL_CONSTRUCTOR);
}
#end
}

@:enum
Expand All @@ -31,6 +43,7 @@ abstract VariableInitialisationCheckTests(String) to String {
var TEST2 = "
abstractAndClass Test {
static inline var TEST:Int = 1;
inline var TEST2:Int = 1;

public function new() {}
}";
Expand All @@ -39,4 +52,16 @@ abstract VariableInitialisationCheckTests(String) to String {
abstract Test(Int) {
var VALUE = 0;
}";
var FINAL = "
abstractAndClass Test {
final VALUE = 0;
}";
var FINAL_CONSTRUCTOR = "
abstractAndClass Test {
final VALUE;

public function new() {
VALUE = 1;
}
}";
}