Add a Linter to the CI to make codestyle more consistent.

.github/workflows/ci-linter.yml

@@ -0,0 +1,20 @@
+name: CI-Lint
+
+on: [push, pull_request]
+
+jobs:
+  build:
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ ubuntu-latest ]
+        smalltalk: [ Squeak64-5.3 ]
+    name: ${{ matrix.smalltalk }} on ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v2
+      - uses: hpi-swa/setup-smalltalkCI@v1
+        with:
+          smalltalk-version: ${{ matrix.smalltalk }}
+      - run: smalltalkci -s ${{ matrix.smalltalk }} .smalltalk.lint.ston
+        shell: bash
+        timeout-minutes: 15

.smalltalk.lint.ston

@@ -0,0 +1,17 @@
+SmalltalkCISpec {
+  #loading : [
+    SCIMetacelloLoadSpec {
+      #baseline : 'SVGMorph',
+      #platforms : [ #squeak ],
+      #directory : 'packages',
+      #load : [ 'linter' ],
+      #useLatestMetacello : true
+    }
+  ],
+  #preLoading : [
+    'scripts/preLoading.st'
+  ],
+  #testing : {
+    #classes : [ #SVGLinterTests ]
+  }
+}

.squot

@@ -3,5 +3,6 @@ OrderedDictionary {
 	'packages/Tests-SVG-Editor.package' : #SquotCypressCodeSerializer,
 	'packages/Tests-SVG-Morphic.package' : #SquotCypressCodeSerializer,
 	'packages/SVG-Editor.package' : #SquotCypressCodeSerializer,
-	'packages/SVG-Morphic.package' : #SquotCypressCodeSerializer
+	'packages/SVG-Morphic.package' : #SquotCypressCodeSerializer,
+	'packages/Tests-SVG-Linter.package' : #SquotCypressCodeSerializer
 }

packages/.filetree

@@ -0,0 +1,2 @@
+{"propertyFileExtension" : ".json",
+"packageExtension" : ".package" }

packages/BaselineOfSVGMorph.package/BaselineOfSVGMorph.class/instance/baseline..st

@@ -6,13 +6,18 @@ baseline: spec
 			spec
 				package: 'SVG-Morphic';
 				package: 'SVG-Editor' with: [spec requires: #('core')];
+				package: 'Tests-SVG-Linter' with: [spec requires: #('core' 'editor' 'swalinter')];
 				package: 'Tests-SVG-Morphic' with: [spec requires: #('core' 'mtf')];
 				package: 'Tests-SVG-Editor' with:  [spec requires: #('editor' 'mtf')];
-				baseline: 'MorphicTestingFramework' with: [spec repository: 'github://hpi-swa-teaching/Morphic-Testing-Framework:master/packages'].
+				baseline: 'MorphicTestingFramework' with: [spec repository: 'github://hpi-swa-teaching/Morphic-Testing-Framework:master/packages'];
+				baseline: 'SwaLint' with: [spec repository: 'github://hpi-swa-teaching/SwaLint:develop/packages'].
 			spec
 				group: 'default' with: #('SVG-Morphic' 'SVG-Editor' 'Tests-SVG-Morphic' 'Tests-SVG-Editor');
 				group: 'core' with: #('SVG-Morphic');
+				group: 'linter' with: #('Tests-SVG-Linter');
 				group: 'editor' with: #('SVG-Editor');
-				group: 'tests' with: #('Tests-SVG-Morphic' 'Tests-SVG-Editor')];
+				group: 'tests' with: #('Tests-SVG-Morphic' 'Tests-SVG-Editor');
 				group: 'mtf' with: #('MorphicTestingFramework');
+        group: 'swalinter' with: #('SwaLint');
 				yourself .
+]

packages/Tests-SVG-Linter.package/.filetree

@@ -0,0 +1,4 @@
+{
+	"noMethodMetaData" : true,
+	"separateMethodMetaAndSource" : false,
+	"useCypressPropertiesFile" : true }

packages/Tests-SVG-Linter.package/.squot-contents

@@ -0,0 +1,5 @@
+SquotTrackedObjectMetadata {
+	#objectClassName : #PackageInfo,
+	#objectsReplacedByNames : true,
+	#serializer : #SquotCypressCodeSerializer
+}

packages/Tests-SVG-Linter.package/SVGLinterTests.class/README.md

@@ -0,0 +1 @@
+LinterTests is the PR validation mechanism used to enforce linter compliance before merge. Individual rules are added as tests and required to have zero offenses.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/arbitraryCodeRule..st

@@ -0,0 +1,4 @@
+private
+arbitraryCodeRule: aRule
+
+	self arbitraryCodeRule: aRule plugIn: SLSmallLintPlugIn.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/arbitraryCodeRule.plugIn..st

@@ -0,0 +1,16 @@
+private
+arbitraryCodeRule: aRule plugIn: aPlugIn
+
+	| failures failingTestObjects result |
+
+	failingTestObjects := OrderedCollection new.
+	failures := ((SLTestRunner new environment: (self environmentFor: aPlugIn withRule: aRule))
+		runOnTests: {aRule}
+		andTestObjects: self classTestObjects) select: [:testObject |
+		result := testObject resultOf: aRule.
+		result isSummary
+			ifTrue: [
+				failingTestObjects addAll: (testObject methods select: [:each | result summarizingCondition value: (each resultOf: result summarizedTest)]).
+				result > 0]
+			ifFalse: [result isNegative]].
+	self assert: failures isEmpty description: 'Failures in: ', failures asString

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/classTestObjects..st

@@ -0,0 +1,4 @@
+accessing
+classTestObjects: aCollection
+
+	classTestObjects := aCollection.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/classTestObjects.st

@@ -0,0 +1,4 @@
+accessing
+classTestObjects
+
+	^ classTestObjects

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/classes..st

@@ -0,0 +1,4 @@
+accessing
+classes: aCollection
+
+	classes := aCollection.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/classes.st

@@ -0,0 +1,4 @@
+accessing
+classes
+
+	^ classes

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/environmentFor.withRule..st

@@ -0,0 +1,6 @@
+private
+environmentFor: aPlugIn withRule: aRule
+
+	^ aPlugIn = SLSmallLintPlugIn
+		ifTrue: [SVGScopedEnvironment newFor: self classTestObjects test: aRule plugIn: SLSmallLintPlugIn]
+		ifFalse: [SLDefaultEnvironment new]

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/hasTrailingWhitespaces..st

@@ -0,0 +1,4 @@
+private
+hasTrailingWhitespaces: aString
+
+	^ (self isLineWithOnlyTabsAndSpaces: aString) not and: [aString last = Character space or: [aString last = Character tab]]

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/initialize.st

@@ -0,0 +1,10 @@
+initialization
+initialize
+
+	super initialize.
+
+	self
+		classes: ((SystemNavigation default allClasses
+			select: [:aClass |
+				(aClass class category beginsWith: 'SVG')]));
+		classTestObjects: (self classes collect: [:anObject | anObject as: SLTestObject]).

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/isLineWithOnlyTabsAndSpaces..st

@@ -0,0 +1,4 @@
+private
+isLineWithOnlyTabsAndSpaces: aString
+
+	^ aString isEmpty or: [aString allSatisfy: [:aChar | aChar = Character space or: [aChar = Character tab]]]

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/methodTestObjects.st

@@ -0,0 +1,4 @@
+accessing
+methodTestObjects
+
+	^ (self classTestObjects collect: [:aClassTestObject | aClassTestObject methods]) flatten

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/methodsLinesDo..st

@@ -0,0 +1,11 @@
+private
+methodsLinesDo: aBlock
+
+	self methodTestObjects do: [:aSLMethodTestObject |
+		| lines |
+		lines := aSLMethodTestObject sourceCode string lines.
+		((lines size >= 2) and: [lines second includesSubstring: '"@linter-ignore"'])
+			ifFalse: [
+				aBlock value: lines
+			]
+	]

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/methodsLinesDo.excludeClasses..st

@@ -0,0 +1,9 @@
+private
+methodsLinesDo: aBlock excludeClasses: aCollection
+
+	self methodTestObjects do: [:aSLMethodTestObject |
+		(aCollection includes: (aSLMethodTestObject parent testClass))
+			ifFalse: [
+				aBlock value: aSLMethodTestObject sourceCode string lines
+			]
+		]

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testAssertOrder.st

@@ -0,0 +1,15 @@
+tests
+testAssertOrder
+
+	"Checks if the arguments of assert: equals: are passed in the right order.
+	A Constant should be the first argument, otherwise the arguments appear
+	in the wrong order in the title of the debugger error message."
+	self methodsLinesDo: [:lines |
+		lines do: [:line |
+			(line matchesRegex: '.*self assert\: .*equals\: .*') ifTrue: [
+				| firstWordAfterEquals |
+				firstWordAfterEquals := (((line splitBy: 'equals: ') second) splitBy: ' ') first.
+				[self assert: ((Compiler evaluate: firstWordAfterEquals) isNil).] on: Error do: ['']
+			]
+		]
+	]

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testCorrectClassPrefix.st

@@ -0,0 +1,18 @@
+tests
+testCorrectClassPrefix
+
+	self classes do: [:aClass |
+		(aClass class category endsWith: 'Client-Core')
+			ifTrue: [self assert: (aClass class name beginsWith: 'TCC')].
+
+		(aClass class category endsWith: 'Client-UI')
+			ifTrue: [self assert: (aClass class name beginsWith: 'TCU')].
+
+		(aClass class category endsWith: 'Tests-Core')
+			ifTrue: [self assert: (aClass class name beginsWith: 'TCTC')].
+
+		(aClass class category endsWith: 'Tests-UI')
+			ifTrue: [self assert: (aClass class name beginsWith: 'TCTU')].
+
+		(aClass class category endsWith: 'Tests-Misc')
+			ifTrue: [self assert: (aClass class name beginsWith: 'TCTM')]].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testDefaultValuesOnClassSide.st

@@ -0,0 +1,8 @@
+tests
+testDefaultValuesOnClassSide
+
+	self methodTestObjects
+		reject: [:method | method methodName = 'defaultTimeout']
+		thenDo: [:method | (method methodName beginsWith: 'default') ifTrue:
+			[self assert: method classSide]
+		].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testIfTrueReturnsRule.st

@@ -0,0 +1,5 @@
+tests
+testIfTrueReturnsRule
+
+	"replace IfTrue ifFalse branches with the shorter bool equivalent if possible"
+	self arbitraryCodeRule: #ifTrueReturnsRule: plugIn: SLRBImportPlugIn.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testInstanceVarAccess.st

@@ -0,0 +1,4 @@
+tests
+testInstanceVarAccess
+
+	self arbitraryCodeRule: #instanceVariableAccessIsConsistent: plugIn: SLSwaMetricsPlugIn.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testLongMethods.st

@@ -0,0 +1,4 @@
+tests
+testLongMethods
+
+	self methodsLinesDo: [:lines | self assert: lines size <= 20]

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testMethodHasEmptyLine.st

@@ -0,0 +1,5 @@
+tests
+testMethodHasEmptyLine
+
+	"also rejects method with tabs or spaces in second line"
+	self methodsLinesDo: [:lines | self assert: (lines size < 2 or: [self isLineWithOnlyTabsAndSpaces: lines second])].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testMethodNameIsLowerCase.st

@@ -0,0 +1,9 @@
+tests
+testMethodNameIsLowerCase
+
+	self methodsLinesDo: [:lines |
+		| firstChar |
+		firstChar := lines first first.
+		firstChar isLetter
+			ifTrue: [self assert: firstChar isLowercase]
+		].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testMethodNoConsecutiveEmptyLines.st

@@ -0,0 +1,7 @@
+tests
+testMethodNoConsecutiveEmptyLines
+
+	self methodsLinesDo: [:lines |
+		1 to: lines size -1 do: [:index |
+			self assert: ((self isLineWithOnlyTabsAndSpaces: (lines at: index))
+				and: [self isLineWithOnlyTabsAndSpaces: (lines at: index + 1)]) not]].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testMethodNoEmptyLineAtEnd.st

@@ -0,0 +1,4 @@
+tests
+testMethodNoEmptyLineAtEnd
+
+	self methodsLinesDo: [:lines | self assert: (self isLineWithOnlyTabsAndSpaces: lines last) not].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testMethodNoTrailingWhitespaces.st

@@ -0,0 +1,5 @@
+tests
+testMethodNoTrailingWhitespaces
+
+	self methodsLinesDo: [:lines |
+		lines do: [:aLine | self assert: (self hasTrailingWhitespaces: aLine) not]].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testMethodNoTwoWhitespacesNextToEachOther.st

@@ -0,0 +1,5 @@
+tests
+testMethodNoTwoWhitespacesNextToEachOther
+
+	self methodsLinesDo: [:lines |
+		lines do: [:aLine | self assert: (aLine includesSubstring: String space, String space) not]].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testMethodParamsHaveMeaningfulNames.st

@@ -0,0 +1,4 @@
+tests
+testMethodParamsHaveMeaningfulNames
+
+	self methodsLinesDo: [:lines | self assert: (lines first includesSubstring: 'anObject') not].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testNoClassComments.st

@@ -0,0 +1,4 @@
+tests
+testNoClassComments
+
+	self arbitraryCodeRule: #noClassComments: plugIn: SLSwaMetricsPlugIn.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testNoDotAfterReturn.st

@@ -0,0 +1,10 @@
+tests
+testNoDotAfterReturn
+
+	| code |
+
+	self methodTestObjects do: [:aSLMethodTestObject |
+		code := aSLMethodTestObject sourceCode string.
+
+		"may return nil, so deny: is not an option"
+		self assert: (SLCodingStylesPlugIn new hasDotAfterReturn: code) ~= true].

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testNoUnclassifiedMethods.st

@@ -0,0 +1,5 @@
+tests
+testNoUnclassifiedMethods
+
+	"all methods have to be classified"
+	self arbitraryCodeRule: #smallLintUnclassifiedMethods.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testNoUnconditionalRecursion.st

@@ -0,0 +1,5 @@
+tests
+testNoUnconditionalRecursion
+
+	"Infinite recursion is forbidden"
+	self arbitraryCodeRule: #smallLintUnconditionalRecursion.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testSmallLintContains.st

@@ -0,0 +1,5 @@
+tests
+testSmallLintContains
+
+	"Use contains instead of detect: ifNone:"
+	self arbitraryCodeRule: #smallLintContains.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testTemporaryVariableCapitalization.st

@@ -0,0 +1,5 @@
+tests
+testTemporaryVariableCapitalization
+
+	"temporary variables should start with lowercase letters"
+	self arbitraryCodeRule: #smallLintTemporaryVariableCapitalization.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testsmallLintClassVariableCapitalization.st

@@ -0,0 +1,5 @@
+tests
+testsmallLintClassVariableCapitalization
+
+	"Class vars are pascalcase"
+	self arbitraryCodeRule: #smallLintClassVariableCapitalization.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testsmallLintCodeCruftLeftInMethods.st

@@ -0,0 +1,5 @@
+tests
+testsmallLintCodeCruftLeftInMethods
+
+	"remove debugger breaks etc from code, otherwise this test fails"
+	self arbitraryCodeRule: #smallLintCodeCruftLeftInMethods.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testsmallLintCollectSelectNotUsed.st

@@ -0,0 +1,5 @@
+tests
+testsmallLintCollectSelectNotUsed
+
+	"prefer collect and select to manual list manipulation and aggregation"
+	self arbitraryCodeRule: #smallLintCollectSelectNotUsed.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testsmallLintConsistencyCheck.st

@@ -0,0 +1,5 @@
+tests
+testsmallLintConsistencyCheck
+
+	"isEmpty, isNil and First should be preferred to size = 0, = nil, at: 1"
+	self arbitraryCodeRule: #smallLintConsistencyCheck.

packages/Tests-SVG-Linter.package/SVGLinterTests.class/instance/testsmallLintDefinesEqualNotHash.st

@@ -0,0 +1,5 @@
+tests
+testsmallLintDefinesEqualNotHash
+
+	"Menu items should be translated somehow. will be evergreen until we sell this as a feature"
+	self arbitraryCodeRule: #smallLintDefinesEqualNotHash.