changed messages for Final and ModifierOrder checks (#493)

* changed messages for Final and ModifierOrder checks
* prepare 2.6.1

.github/workflows/checkstyle-linux.yml

@@ -15,7 +15,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        haxe-version: ['3.4.7', '4.0.3', 'nightly']
+        haxe-version: ['3.4.7', '4.0.5', 'nightly']
     env:
       CC_TEST_REPORTER_ID: 1dff6f89d7179dff5db635c6b4fe64acdd5694c9ed44d7da5f12f0f7d3d163b7
       CODECOV_TOKEN: ${{secrets.CODECOV_TOKEN}}
@@ -26,7 +26,7 @@ jobs:
       with:
         node-version: 10
     - name: Installing codeclimate client
-      if: matrix.haxe-version == '4.0.3'
+      if: matrix.haxe-version == '4.0.5'
       run: |
         curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter
         chmod +x ./cc-test-reporter
@@ -68,13 +68,13 @@ jobs:
     - name: Run Java tests
       run: npx haxe testJava.hxml
     - name: Format and upload codeclimate coverage
-      if: success() && matrix.haxe-version == '4.0.3'
+      if: success() && matrix.haxe-version == '4.0.5'
       run: |
         ( \
           cd src; \
           ../cc-test-reporter format-coverage -t lcov ../lcov.info; \
           ../cc-test-reporter upload-coverage; \
         )
     - name: Upload results to codecov
-      if: success() && matrix.haxe-version == '4.0.3'
+      if: success() && matrix.haxe-version == '4.0.5'
       run: bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"

.haxerc

@@ -1,4 +1,4 @@
 {
-  "version": "4.0.3",
+  "version": "4.0.5",
   "resolveLibs": "scoped"
 }

.travis.yml

@@ -17,7 +17,7 @@ install:
   - npm install
   - if [[ "$HAXE_VERSION" == "haxe347" ]]; then mv haxe_libraries haxe4_libraries; mv haxe3_libraries haxe_libraries; fi
   - if [[ "$HAXE_VERSION" == "haxe347" ]]; then npx lix download haxe 3.4.7; npx lix use haxe 3.4.7; fi
-  - if [[ "$HAXE_VERSION" == "haxe4" ]]; then npx lix download haxe 4.0.3; npx lix use haxe 4.0.3; fi
+  - if [[ "$HAXE_VERSION" == "haxe4" ]]; then npx lix download haxe 4.0.5; npx lix use haxe 4.0.5; fi
   - if [[ "$HAXE_VERSION" == "nightly" ]]; then npx lix download haxe nightly; npx lix use haxe nightly; fi
   - npx lix download
   - npx haxe -version

CHANGELOG.md

@@ -2,7 +2,11 @@
 
 ## dev branch / next version (2.x.x)
 
+## version 2.6.1 (2019-12-17)
+
 - Added `allowFinal` setting to `VariableInitialisation`, fixes [#491](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/491) ([#492](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/492))
+- Changed message of `Final` check when detecting `public static var` ([#493](https://github.com/HaxeCheckstyle/haxe-checkstyle/issues/493))
+- Changed message of `ModifierOrder` check to include actual and expected modifier order
 
 ## version 2.6.0 (2019-12-01)
 

haxe3_libraries/test-adapter.hxml

@@ -1,5 +1,5 @@
--D test-adapter=1.2.5
-# @install: lix --silent download "haxelib:/test-adapter#1.2.5" into test-adapter/1.2.5/haxelib
+-D test-adapter=1.2.6
+# @install: lix --silent download "haxelib:/test-adapter#1.2.6" into test-adapter/1.2.6/haxelib
 -lib json2object
--cp ${HAXE_LIBCACHE}/test-adapter/1.2.5/haxelib/
+-cp ${HAXE_LIBCACHE}/test-adapter/1.2.6/haxelib/
 --macro _testadapter.Macro.init()

haxe_libraries/test-adapter.hxml

@@ -1,5 +1,5 @@
--D test-adapter=1.2.5
-# @install: lix --silent download "haxelib:/test-adapter#1.2.5" into test-adapter/1.2.5/haxelib
+-D test-adapter=1.2.6
+# @install: lix --silent download "haxelib:/test-adapter#1.2.6" into test-adapter/1.2.6/haxelib
 -lib json2object
--cp ${HAXE_LIBCACHE}/test-adapter/1.2.5/haxelib/
+-cp ${HAXE_LIBCACHE}/test-adapter/1.2.6/haxelib/
 --macro _testadapter.Macro.init()

makeReleaseZip.sh

@@ -2,7 +2,7 @@
 
 npm install
 npx lix download
-npx lix use haxe 4.0.3
+npx lix use haxe 4.0.5
 
 npx haxe buildAll.hxml
 

package.json

@@ -1,6 +1,6 @@
 {
     "name": "haxe-checkstyle",
-    "version": "2.6.0",
+    "version": "2.6.1",
     "description": "Automated code analysis ideal for projects that want to enforce a coding standard.",
     "repository": {
         "type": "git",

src/checkstyle/checks/modifier/FinalCheck.hx

@@ -27,20 +27,22 @@ class FinalCheck extends Check {
 		}
 		if (f.access.contains(AFinal)) return;
 
-		checkPublicStatic(f);
-		checkInlineVar(f);
+		if (checkInlineVar(f)) {
+			logPos('Consider using "inline final" for field "${f.name}"', f.pos, USE_INLINE_FINAL);
+			return;
+		}
+		if (checkPublicStatic(f)) {
+			logPos('Consider making public static field "${f.name}" "final" or "private"', f.pos, SHOULD_BE_PUBLIC_FINAL);
+			return;
+		}
 	}
 
-	function checkPublicStatic(f:Field) {
-		if (!f.access.contains(APublic)) return;
-		if (!f.access.contains(AStatic)) return;
-
-		logPos('Public static field "${f.name}" should be "final"', f.pos, SHOULD_BE_PUBLIC_FINAL);
+	function checkPublicStatic(f:Field):Bool {
+		return (f.access.contains(APublic)) && (f.access.contains(AStatic));
 	}
 
-	function checkInlineVar(f:Field) {
-		if (!f.access.contains(AInline)) return;
-		logPos('Consider using "inline final" for field "${f.name}"', f.pos, USE_INLINE_FINAL);
+	function checkInlineVar(f:Field):Bool {
+		return (f.access.contains(AInline));
 	}
 
 	override public function detectableInstances():DetectableInstances {

src/checkstyle/checks/modifier/ModifierOrderCheck.hx

@@ -30,16 +30,30 @@ class ModifierOrderCheck extends Check {
 		var lastIndex:Int = -1;
 		var index:Int;
 
+		var actual:Array<String> = [];
+		var expected:Array<String> = [];
+		#if haxe4
+		expected.resize(modifiers.length);
+		#else
+		for (mod in modifiers) expected.push(null);
+		#end
+
+		var compliant:Bool = true;
+
 		for (access in f.access) {
 			var modifier:ModifierOrderCheckModifier = access;
 			index = modifiers.indexOf(modifier);
+			if (index < 0) continue;
+			actual.push(ModifierOrderCheckModifier.accessToString(access));
+			expected[index] = ModifierOrderCheckModifier.accessToString(access);
 			if (index < lastIndex) {
-				var pos = calcPos(f);
-				warnOrder(f.name, modifier, pos);
-				return;
+				compliant = false;
 			}
 			lastIndex = index;
 		}
+		if (compliant) return;
+		var pos = calcPos(f);
+		warnOrder(f.name, actual, expected, pos);
 	}
 
 	function calcPos(f:Field):Position {
@@ -54,8 +68,9 @@ class ModifierOrderCheck extends Check {
 		}
 	}
 
-	function warnOrder(name:String, modifier:ModifierOrderCheckModifier, pos:Position) {
-		logPos('"${name}" modifier order is invalid (modifier: "${modifier}")', pos);
+	function warnOrder(name:String, actual:Array<String>, expected:Array<String>, pos:Position) {
+		expected = expected.filter(function(f:String):Bool return (f != null));
+		logPos('modifier order for field "${name}" is "${actual.join(" ")}" but should be "${expected.join(" ")}"', pos);
 	}
 }
 
@@ -98,4 +113,20 @@ abstract ModifierOrderCheckModifier(String) {
 			#end
 		}
 	}
+
+	public static function accessToString(access:Access):String {
+		return switch (access) {
+			case APublic: "public";
+			case APrivate: "private";
+			case AStatic: "static";
+			case AInline: "inline";
+			case AOverride: "override";
+			case AMacro: "macro";
+			case ADynamic: "dynamic";
+			#if haxe4
+			case AExtern: "extern";
+			case AFinal: "final";
+			#end
+		}
+	}
 }

test/checkstyle/checks/modifier/FinalCheckTest.hx

@@ -3,7 +3,7 @@ package checkstyle.checks.modifier;
 #if haxe4
 class FinalCheckTest extends CheckTestCase<FinalCheckTests> {
 	static inline var ERROR_INLINE_VAR:String = 'Consider using "inline final" for field "test"';
-	static inline var ERROR_PUBLIC_STATIC:String = 'Public static field "test" should be "final"';
+	static inline var ERROR_PUBLIC_STATIC:String = 'Consider making public static field "test" "final" or "private"';
 
 	@Test
 	public function testInlineFinal() {
@@ -27,6 +27,9 @@ abstract FinalCheckTests(String) to String {
 		public inline final test:String = '0';
 		public static final test2:String = '0';
 		public static var test3(default, null):String = '0';
+		private static var test4:String = '0';
+		public var test5:String = '0';
+		private var test5:String = '0';
 		final function test2() {
 		}
 	}";

test/checkstyle/checks/modifier/ModifierOrderCheckTest.hx

@@ -1,8 +1,6 @@
 package checkstyle.checks.modifier;
 
 class ModifierOrderCheckTest extends CheckTestCase<ModifierOrderCheckTests> {
-	static inline var ERROR:String = '"test" modifier order is invalid (modifier: "PUBLIC_PRIVATE")';
-
 	@Test
 	public function testCorrectOrder() {
 		var check = new ModifierOrderCheck();
@@ -15,19 +13,19 @@ class ModifierOrderCheckTest extends CheckTestCase<ModifierOrderCheckTests> {
 	@Test
 	public function testWrongOrder() {
 		var check = new ModifierOrderCheck();
-		assertMsg(check, TEST2, '"test" modifier order is invalid (modifier: "OVERRIDE")');
-		assertMsg(check, TEST3, '"test" modifier order is invalid (modifier: "STATIC")');
-		assertMsg(check, TEST4, '"test" modifier order is invalid (modifier: "MACRO")');
-		assertMsg(check, TEST5, ERROR);
-		assertMsg(check, TEST7, ERROR);
-		assertMsg(check, TEST8, ERROR);
+		assertMsg(check, TEST2, 'modifier order for field "test" is "public override" but should be "override public"');
+		assertMsg(check, TEST3, 'modifier order for field "test" is "public inline static" but should be "public static inline"');
+		assertMsg(check, TEST4, 'modifier order for field "test" is "public macro" but should be "macro public"');
+		assertMsg(check, TEST5, 'modifier order for field "test" is "dynamic public" but should be "public dynamic"');
+		assertMsg(check, TEST7, 'modifier order for field "test" is "inline public" but should be "public inline"');
+		assertMsg(check, TEST8, 'modifier order for field "test" is "inline public" but should be "public inline"');
 	}
 
 	@Test
 	public function testModifiers() {
 		var check = new ModifierOrderCheck();
 		check.modifiers = [DYNAMIC, PUBLIC_PRIVATE, OVERRIDE, INLINE, STATIC, MACRO];
-		assertMsg(check, TEST1, '"test6" modifier order is invalid (modifier: "INLINE")');
+		assertMsg(check, TEST1, 'modifier order for field "test6" is "public static inline" but should be "public inline static"');
 		assertNoMsg(check, TEST2);
 		assertNoMsg(check, TEST3);
 		assertNoMsg(check, TEST4);