Skip to content

Commit

Permalink
Merge pull request #835 from ApptiveGrid/CleanCodeTest-add-RuleTests
Browse files Browse the repository at this point in the history
  • Loading branch information
noha authored Oct 10, 2024
2 parents 7e60834 + 108b956 commit 46b6ff2
Showing 1 changed file with 61 additions and 13 deletions.
74 changes: 61 additions & 13 deletions src/Soil-Core-Tests/SoilCleanCodeTest.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,44 @@ SoilCleanCodeTest class >> cleanupWhiteSpaceTrimRight [
refactoring execute ]
]

{ #category : #running }
SoilCleanCodeTest >> assertValidLintRule: aLintRule [
self assertValidLintRule: aLintRule withExceptions: #()
]

{ #category : #running }
SoilCleanCodeTest >> assertValidLintRule: aLintRule withExceptions: someNames [
| runner results |
runner := ReSmalllintChecker new.
runner
rule: {aLintRule};
environment: ( RBBrowserEnvironment new
forClasses: self packageToCheck definedClasses);
run.

results := (runner criticsOf: aLintRule) reject: [ :critique | someNames includes: critique entity name ].
self
assert: results isEmpty
description: [ String
streamContents: [ :s |
s
<< aLintRule rationale;
lf;
<< 'Violations: ';
lf.
results
do: [ :e |
s
<< '- ';
print: e entity ]
separatedBy: [ s lf ] ] ]
]

{ #category : #running }
SoilCleanCodeTest >> packageToCheck [
^ Soil package
]

{ #category : #tests }
SoilCleanCodeTest >> testCodeCoverage [

Expand All @@ -36,7 +74,7 @@ SoilCleanCodeTest >> testCodeCoverage [
"take care: this test interferes with the run coverage feature in the testRunner, we should improve
it to not run test tagged <ignoreForCoverage>"
collector :=Smalltalk globals at: #CoverageCollector ifPresent: [:class | class new] ifAbsent: [self error: 'class CoverageCollector not found' ].
methods := Soil package methods.
methods := self packageToCheck methods.
"Remove all the methods and classes we are not interested in"

"subclassResponsibility methods"
Expand Down Expand Up @@ -72,7 +110,7 @@ SoilCleanCodeTest >> testNoDuplicatedMethodInHierarchy [
"There should be no methods in the hierachy that are already in a superclass"

| methods |
methods := Soil package methods reject: [:method | method isFromTrait].
methods := self packageToCheck methods reject: [:method | method isFromTrait].
methods := methods select: [:method |
method methodClass superclass
ifNotNil: [ :superclass | (superclass lookupSelector: method selector)
Expand All @@ -88,7 +126,7 @@ SoilCleanCodeTest >> testNoDuplicatedMethodInHierarchy [
SoilCleanCodeTest >> testNoShadowedVariablesInMethods [
"Fail if there are methods who define shadowed temps or args"
| found validExceptions remaining |
found := Soil package methods select: [ :m |
found := self packageToCheck methods select: [ :m |
m isQuick not and: [ "quick methods do not define variables"
m ast variableDefinitionNodes anySatisfy: [ :node | node variable isShadowing ] ]].
"No other exceptions beside the ones mentioned here should be allowed"
Expand All @@ -109,8 +147,8 @@ SoilCleanCodeTest >> testNoUncategorizedMethods [

| violating classes |
self skip.
classes := Soil package definedClasses
, (Soil package definedClasses collect: [ :each | each classSide ]).
classes := self packageToCheck definedClasses
, (self packageToCheck definedClasses collect: [ :each | each classSide ]).

violating := classes select: [ :class |
class protocolNames includes: #'as yet unclassified' ].
Expand All @@ -127,7 +165,7 @@ SoilCleanCodeTest >> testNoUnimplementedCalls [
<ignoreNotImplementedSelectors: #(selector:to:be:ignored)>"

self skip.
remaining := Soil package methods select: [ :meth |
remaining := self packageToCheck methods select: [ :meth |
| ignored |
ignored := meth allIgnoredNotImplementedSelectors.
meth messages anySatisfy: [ :m |
Expand All @@ -145,8 +183,8 @@ SoilCleanCodeTest >> testNoUnsentMessages [
| found methods|
"As this is not critical, we skip this test. If you are want to cleanup, just run it!"
self skip.
found := Soil package allUnsentMessages.
methods := Soil package methods select: [ :method | found includes: method selector ].
found := self packageToCheck allUnsentMessages.
methods := self packageToCheck methods select: [ :method | found includes: method selector ].
methods := methods reject: [:method | method hasPragmaNamed: #inspectorPresentationOrder:title:
].

Expand All @@ -162,7 +200,7 @@ SoilCleanCodeTest >> testNoUnusedClasses [
| found knownviolations |
"As this is not critical, we skip this test. If you are want to cleanup, just run it!"
self skip.
found := Soil package definedClasses reject: [ :class | class isUsed].
found := self packageToCheck definedClasses reject: [ :class | class isUsed].

knownviolations := #( ).
found := found reject: [:class | knownviolations includes: class name ].
Expand All @@ -175,8 +213,8 @@ SoilCleanCodeTest >> testNoUnusedClasses [
{ #category : #tests }
SoilCleanCodeTest >> testNoUnusedInstanceVariablesLeft [
| variables classes validExceptions remaining |
classes := Soil package definedClasses
, (Soil package definedClasses collect: [ :each | each classSide ]).
classes := self packageToCheck definedClasses
, (self packageToCheck definedClasses collect: [ :each | each classSide ]).

variables := classes flatCollect: [ :each | each instanceVariables ].
variables := variables reject: [ :each | each isReferenced ].
Expand All @@ -193,7 +231,7 @@ SoilCleanCodeTest >> testNoUnusedInstanceVariablesLeft [
SoilCleanCodeTest >> testNoUnusedTemporaryVariablesLeft [
"Fail if there are methods who have unused temporary variables"
| found |
found := Soil package methods select: [ :m |
found := self packageToCheck methods select: [ :m |
m hasTemporaries and: [ m ast temporaries anySatisfy: [ :x | x binding isUsed not] ] ].

self
Expand All @@ -205,9 +243,19 @@ SoilCleanCodeTest >> testNoUnusedTemporaryVariablesLeft [
SoilCleanCodeTest >> testNoVarsDefinedByClassesShadow [
"make sure no class vars or ivars shadow"
| classes |
classes := Soil package definedClasses select: [ :class |
classes := self packageToCheck definedClasses select: [ :class |
class definedVariables anySatisfy: [ :var |
var isShadowing ] ].

self assert: classes isEmpty description: classes asArray asString
]

{ #category : #tests }
SoilCleanCodeTest >> testReDoNotSendSuperInitializeInClassSideRule [
self assertValidLintRule: ReDoNotSendSuperInitializeInClassSideRule new
]

{ #category : #tests }
SoilCleanCodeTest >> testReInstanceVariableCapitalizationRule [
self assertValidLintRule: ReInstanceVariableCapitalizationRule new
]

0 comments on commit 46b6ff2

Please sign in to comment.