Merge pull request #1567 from moosetechnology/java-dead-code-rule

Update/extend Java dead code rule (with tests)

Author: NicolasAnquetil

src/MooseIDE-DeadCode-Tests/MiDeadCodeBrowserModelTest.class.st

@@ -200,6 +200,7 @@ MiDeadCodeBrowserModelTest >> testIsNewDead [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaAnnotationHeuristicDeadNoAnnotation [
+	"Unused method with no annotation is dead"
 
 	| entity |
 	entity := FamixJavaMethod new.
@@ -209,6 +210,7 @@ MiDeadCodeBrowserModelTest >> testJavaAnnotationHeuristicDeadNoAnnotation [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaAnnotationHeuristicDeadWrongAnnotation [
+	"Unused method with @Deprecated annotation is dead"
 
 	| entity |
 	entity := self javaMethod: 'someFunctionality' annotatedWith: 'Deprecated'.
@@ -218,6 +220,7 @@ MiDeadCodeBrowserModelTest >> testJavaAnnotationHeuristicDeadWrongAnnotation [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaAnnotationHeuristicNotDead [
+	"Unused method with JUnit annotation is not dead"
 
 	| entity |
 	entity := self javaMethod: 'someFunctionality' annotatedWith: 'BeforeClass'.
@@ -227,6 +230,7 @@ MiDeadCodeBrowserModelTest >> testJavaAnnotationHeuristicNotDead [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDead [
+	"Unused method (entity1.doSomething()) with sub-method declaring @Override is not dead"
 
 	| entity1 entity2 annotation |
 	entity1 := self javaClassMethod: 'doSomething()'.
@@ -247,6 +251,8 @@ MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDead [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDeadNoAnnotation [
+	"Unused method (entity1.doSomething()) with sub-method not declaring @Override is dead"
+
 	| entity1 entity2 |
 	entity1 := self javaClassMethod: 'doSomething()'.
 	entity2 := self javaClassMethod: 'doSomething()'.
@@ -257,6 +263,8 @@ MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDeadNoAnnotation [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDeadNoOverride [
+	"Unused method (entity1.doSomething()) with sub-method not declaring @Override is dead"
+
 	| entity1 entity2 annotation |
 
 	entity1 := self javaClassMethod: 'doSomething()'.
@@ -279,6 +287,9 @@ MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDeadNoOverride [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDeadNoSubMethod [
+	"Unused method (entity1.doSomething()) with sub-method declaring @Override is dead
+	 (note: entity1.doSomething() is not a sub method of entity1.doSomething()"
+
 	| entity1 entity2 annotation |
 
 	entity1 := self javaClassMethod: 'doSomething()'.
@@ -298,26 +309,55 @@ MiDeadCodeBrowserModelTest >> testJavaHasOverridingHeuristicDeadNoSubMethod [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaImplementsHeuristicNoInterface [
+	"Unused method (entity1.doSomething()) not implementing an abtract declaration is dead"
 
 	| entity1 |
 	entity1 := self javaClassMethod: 'doSomething()'.
 
-	self assert: (MiDeadCodeJavaImplementsInterfaceHeuristic new isDead: entity1)
+	self assert: (MiDeadCodeJavaImplementsAbstractHeuristic new isDead: entity1)
 ]
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaImplementsHeuristicNoOverride [
+	"Unused method (entity1.doSomething())) implementing an interface declaration is not dead
+	 (according to MiDeadCodeJavaImplementsAbstractHeuristic)"
 
 	| entity1 entity2 |
 	entity1 := self javaClassMethod: 'doSomething()'.
 	entity2 := self javaInterfaceMethod: 'doSomething()'.
 	self javaClass: entity1 parentType implements: entity2 parentType.
 
-	self deny: (MiDeadCodeJavaImplementsInterfaceHeuristic new isDead: entity1)
+	self deny: (MiDeadCodeJavaImplementsAbstractHeuristic new isDead: entity1)
+]
+
+{ #category : 'tests - heuristics' }
+MiDeadCodeBrowserModelTest >> testJavaImplementsHeuristicNotAbstract [
+	"Unused method (entity1.doSomething()) overriding a super-method is dead"
+
+	| entity1 entity2 |
+	entity1 := self javaClassMethod: 'doSomething()'.
+	entity2 := self javaClassMethod: 'doSomething()'.
+	self javaClass: entity1 parentType inherits: entity2 parentType.
+
+	self assert: (MiDeadCodeJavaImplementsAbstractHeuristic new isDead: entity1)
+]
+
+{ #category : 'tests - heuristics' }
+MiDeadCodeBrowserModelTest >> testJavaImplementsHeuristicWithAbstract [
+	"Unused method (entity1.doSomething()) overriding an abstract super-method is not dead"
+
+	| entity1 entity2 |
+	entity1 := self javaClassMethod: 'doSomething()'.
+	entity2 := self javaClassMethod: 'doSomething()'.
+	entity2 isAbstract: true.
+	self javaClass: entity1 parentType inherits: entity2 parentType.
+
+	self deny: (MiDeadCodeJavaImplementsAbstractHeuristic new isDead: entity1)
 ]
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaImplementsHeuristicWithOverride [
+	"Unused method (entity1.doSomething()) implementing an interface declaration but overriding a concrete implementation is dead"
 
 	| entity1 entity2 entity3 |
 	entity1 := self javaClassMethod: 'doSomething()'.
@@ -326,11 +366,14 @@ MiDeadCodeBrowserModelTest >> testJavaImplementsHeuristicWithOverride [
 	self javaClass: entity1 parentType inherits: entity2 parentType.
 	self javaClass: entity1 parentType implements: entity3 parentType.
 
-	self assert: (MiDeadCodeJavaImplementsInterfaceHeuristic new isDead: entity1)
+	self assert: (MiDeadCodeJavaImplementsAbstractHeuristic new isDead: entity1)
 ]
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaOverridesHeuristicDeadNoInterface [
+	"Unused method (entity1.doSomething()) is dead
+	 note: no link between entity1 and entity2"
+
 	| entity1 entity2 |
 	entity1 := self javaClassMethod: 'doSomething()'.
 	entity2 := self javaInterfaceMethod: 'doSomething()'.
@@ -341,6 +384,8 @@ MiDeadCodeBrowserModelTest >> testJavaOverridesHeuristicDeadNoInterface [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaOverridesHeuristicDeadNoInvocation [
+	"Unused method implementing an interface declaration is dead
+	 (according to MiDeadCodeJavaOverridesInvokedHeuristic)"
 
 	| entity1 entity2 |
 	entity1 := self javaClassMethod: 'doSomething()'.
@@ -352,6 +397,9 @@ MiDeadCodeBrowserModelTest >> testJavaOverridesHeuristicDeadNoInvocation [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaOverridesHeuristicDeadNoMethod [
+	"Unused method (entity1.doSomething()) with no implementing sub-method used is dead
+	 (according to MiDeadCodeJavaOverridesInvokedHeuristic)
+	 note: methods names are different"
 
 	| entity1 entity2 |
 	entity1 := self javaClassMethod: 'doSomething()'.
@@ -364,6 +412,8 @@ MiDeadCodeBrowserModelTest >> testJavaOverridesHeuristicDeadNoMethod [
 
 { #category : 'tests - heuristics' }
 MiDeadCodeBrowserModelTest >> testJavaOverridesHeuristicNotDead [
+	"Unused method (entity1.doSomething()) with implementing sub-method used is not dead
+	 (according to MiDeadCodeJavaOverridesInvokedHeuristic)"
 
 	| entity1 entity2 |
 	entity1 := self javaClassMethod: 'doSomething()'.
@@ -572,11 +622,11 @@ MiDeadCodeBrowserModelTest >> testRecursionRoundNotAddingOutsideFollwedEntities
 MiDeadCodeBrowserModelTest >> testSortHeuristics [
 
 	model chooseHeuristics: {
-		MiDeadCodeJavaImplementsInterfaceHeuristic new .
+		MiDeadCodeJavaImplementsAbstractHeuristic new .
 		MiDeadCodeJavaTestAnnotationHeuristic new }.
 
 	self assert: model chosenHeuristics size equals: 3.
-	self assert: model chosenHeuristics first  class equals: MiDeadCodeJavaImplementsInterfaceHeuristic.
+	self assert: model chosenHeuristics first  class equals: MiDeadCodeJavaImplementsAbstractHeuristic.
 	self assert: model chosenHeuristics second class equals: MiDeadCodeJavaTestAnnotationHeuristic.
 	self assert: model chosenHeuristics third  class equals: MiDeadCodeNoIncomingInvocationHeuristic.
 ]

src/MooseIDE-DeadCode/MiDeadCodeAbstractHeuristic.class.st

@@ -23,7 +23,7 @@ Class {
 { #category : 'testing' }
 MiDeadCodeAbstractHeuristic class >> isAbstract [
 
-	^self name includesSubstring: 'Abstract'
+	^self name includesSubstring: 'MiDeadCodeAbstract'
 ]
 
 { #category : 'accessing' }

src/MooseIDE-DeadCode/MiDeadCodeJavaHasOverridingHeuristic.class.st

@@ -1,3 +1,8 @@
+"
+A method having an ""@Override"" sub-method (in a subclass) is assumed not-dead,
+because one cannot remove the overriden method
+(@Override makes it mandatory to have a super-method)
+"
 Class {
 	#name : 'MiDeadCodeJavaHasOverridingHeuristic',
 	#superclass : 'MiDeadCodeAbstractJavaHeuristic',

src/MooseIDE-DeadCode/MiDeadCodeJavaImplementsAbstractHeuristic.class.st

@@ -0,0 +1,80 @@
+"
+A heuristic to recognize methods overriding another method that is called.
+
+If a method in a super-class (or interface) is called, we assume any method implementing it in a concrete class can be called
+"
+Class {
+	#name : 'MiDeadCodeJavaImplementsAbstractHeuristic',
+	#superclass : 'MiDeadCodeAbstractJavaHeuristic',
+	#category : 'MooseIDE-DeadCode-Rules',
+	#package : 'MooseIDE-DeadCode',
+	#tag : 'Rules'
+}
+
+{ #category : 'testing' }
+MiDeadCodeJavaImplementsAbstractHeuristic >> any: tWithMethods declaresMethod: aTMethod [
+
+	tWithMethods do: [ :aTWithMethods |
+		aTWithMethods methods
+			detect: [ :mth | mth signature = aTMethod signature ]
+			ifOne: [ :mth | ^true ]
+	].
+
+	^false
+]
+
+{ #category : 'testing' }
+MiDeadCodeJavaImplementsAbstractHeuristic >> collect: tWithMethods definedMethod: aTMethod [
+
+	^tWithMethods
+		collect: [ :aTWithMethods |
+			aTWithMethods methods
+				detect: [ :mth | mth signature = aTMethod signature ]
+				ifNone: [ nil ]
+		]
+		thenReject: #isNil
+
+]
+
+{ #category : 'accessing' }
+MiDeadCodeJavaImplementsAbstractHeuristic >> description [
+
+	^super description ,
+	'A method implementing an abstract method, even dead, is needed for the implementation'
+]
+
+{ #category : 'accessing' }
+MiDeadCodeJavaImplementsAbstractHeuristic >> name [
+
+	^super name , 'Implements abstract method'
+]
+
+{ #category : 'testing' }
+MiDeadCodeJavaImplementsAbstractHeuristic >> notDead: aTMethod [
+	"- Get all methods defined in super-classes
+	   - if any one is not abstract, then return false, the rule does not apply
+	   - if there is one, it must be abstract, return true (is not dead)
+	 - Look at all methods defined in implemented interface
+	   if any matches this method then return true (is not dead)"
+
+	| superclasses definedMethods implementedInterfaces |
+
+	superclasses := aTMethod parentType superclassHierarchy
+		reject: #isNil. "protect against a bug in VerveineJ"
+
+	definedMethods := self collect: superclasses definedMethod: aTMethod.
+	
+	definedMethods
+		detect: [ :definedMethod | definedMethod isAbstract not ]
+		ifOne: [ :definedMethod | ^false ].
+
+	definedMethods ifNotEmpty: [ ^true ].
+
+	implementedInterfaces := ({ aTMethod parentType } , superclasses)
+		flatCollectAsSet: [ :c | self implementedInterfaceHierarchy: c ].
+
+	(self any: implementedInterfaces declaresMethod: aTMethod)
+		ifTrue: [ ^true ].
+
+	^ false
+]