Implement shadowing of globals and add tests

Also fix a bug when we do not create a new global if a previous one of the same name got shadowed
moosetechnology · Nov 14, 2024 · d807eb7 · d807eb7
1 parent 05cf9c9
commit d807eb7
Show file tree

Hide file tree

Showing 5 changed files with 116 additions and 73 deletions.
diff --git a/src/Famix-Python-Entities/FamixPythonEntity.class.st b/src/Famix-Python-Entities/FamixPythonEntity.class.st
@@ -163,6 +163,13 @@ FamixPythonEntity >> isReference [
 	^ false
 ]
 
+{ #category : 'testing' }
+FamixPythonEntity >> isShadowable [
+
+	<generated>
+	^ false
+]
+
 { #category : 'testing' }
 FamixPythonEntity >> isStructuralEntity [
 

diff --git a/src/Famix-Python-Entities/FamixPythonGlobalVariable.class.st b/src/Famix-Python-Entities/FamixPythonGlobalVariable.class.st
@@ -18,6 +18,8 @@
 |---|
 | `declaredType` | `FamixTTypedEntity` | `typedEntities` | `FamixTType` | Type of the entity, if any|
 | `receivingInvocations` | `FamixTInvocationsReceiver` | `receiver` | `FamixTInvocation` | List of invocations performed on this entity (considered as the receiver)|
+| `shadowedEntity` | `FamixTShadower` | `shadowingEntity` | `FamixTShadowable` | Entity that is been shadowed by myself in my defining scope.|
+| `shadowingEntity` | `FamixTShadowable` | `shadowedEntity` | `FamixTShadower` | Entity shadowing me in my defining scope.|
 | `sourceAnchor` | `FamixTSourceEntity` | `element` | `FamixTSourceAnchor` | SourceAnchor entity linking to the original source code for this entity|
 
 
@@ -33,8 +35,8 @@
 Class {
 	#name : 'FamixPythonGlobalVariable',
 	#superclass : 'FamixPythonNamedEntity',
-	#traits : 'FamixTGlobalVariable + FamixTImportable + FamixTInvocationsReceiver',
-	#classTraits : 'FamixTGlobalVariable classTrait + FamixTImportable classTrait + FamixTInvocationsReceiver classTrait',
+	#traits : 'FamixTGlobalVariable + FamixTImportable + FamixTInvocationsReceiver + FamixTShadowable + FamixTShadower',
+	#classTraits : 'FamixTGlobalVariable classTrait + FamixTImportable classTrait + FamixTInvocationsReceiver classTrait + FamixTShadowable classTrait + FamixTShadower classTrait',
 	#category : 'Famix-Python-Entities-Entities',
 	#package : 'Famix-Python-Entities',
 	#tag : 'Entities'

diff --git a/src/Famix-Python-Generator/FamixPythonGenerator.class.st b/src/Famix-Python-Generator/FamixPythonGenerator.class.st
@@ -116,6 +116,8 @@ FamixPythonGenerator >> defineHierarchy [
 	globalVariable --|> #TGlobalVariable.
 	globalVariable --|> #TInvocationsReceiver.
 	globalVariable --|> #TImportable.
+	globalVariable --|> #TShadowable.
+	globalVariable --|> #TShadower.
 
 	implicitVariable --|> namedEntity.
 	implicitVariable --|> #TImplicitVariable.

diff --git a/src/Famix-Python-Importer-Tests/FamixPythonProject1Test.class.st b/src/Famix-Python-Importer-Tests/FamixPythonProject1Test.class.st
@@ -171,53 +171,53 @@ FamixPythonProject1Test >> testClassInPackage [
 ]
 
 { #category : 'tests - shadowing' }
-FamixPythonProject1Test >> testClassShadowedByFunction [
+FamixPythonProject1Test >> testClassShadowedByClass [
 
 	| shadower shadowed |
-	shadower := (self model allFunctions select: [ :entity | entity name = 'shadowedName' ]) asOrderedCollection detectMax: [ :entity |
+	shadower := (self model allClasses select: [ :entity | entity name = 'ClassShadowedByOtherClass' ]) asOrderedCollection detectMax: [ :entity |
 		            entity sourceAnchor startPos ].
-	shadowed := (self model allClasses select: [ :entity | entity name = 'shadowedName' ]) asOrderedCollection detectMin: [ :entity |
+	shadowed := (self model allClasses select: [ :entity | entity name = 'ClassShadowedByOtherClass' ]) asOrderedCollection detectMin: [ :entity |
 		            entity sourceAnchor startPos ].
 
-	self assert: shadower name equals: 'shadowedName'.
+	self assert: shadower name equals: 'ClassShadowedByOtherClass'.
 	self deny: shadower isShadowed.
-	self assert: shadower functionOwner equals: (self moduleNamed: 'moduleWithShadowing').
+	self assert: shadower typeContainer equals: (self moduleNamed: 'moduleWithShadowing').
 	self assert: shadower shadowedEntity equals: shadowed.
-	self assert: shadower sourceText equals: ('def shadowedName():
-    print("I''m the last one so I win")' copyReplaceAll: String cr with: String lf).
+	self assert: shadower sourceText equals: ('class ClassShadowedByOtherClass:
 
-	self assert: shadowed name equals: 'shadowedName'.
+    def __init__(self):
+        self.i_var_of_shadowed_class = 3' copyReplaceAll: String cr with: String lf).
+
+	self assert: shadowed name equals: 'ClassShadowedByOtherClass'.
 	self assert: shadowed isShadowed.
 	self assert: shadowed typeContainer equals: (self moduleNamed: 'moduleWithShadowing').
 	self assert: shadowed shadowingEntity equals: shadower.
-	self assert: shadowed sourceText equals: ('class shadowedName:
-    shadowedNameVar = 2' copyReplaceAll: String cr with: String lf)
+	self assert: shadowed sourceText equals: ('class ClassShadowedByOtherClass:
+    c_var_of_shadowed_class = True' copyReplaceAll: String cr with: String lf)
 ]
 
 { #category : 'tests - shadowing' }
-FamixPythonProject1Test >> testClassShadowedByOtherClass [
+FamixPythonProject1Test >> testClassShadowedByFunction [
 
 	| shadower shadowed |
-	shadower := (self model allClasses select: [ :entity | entity name = 'ClassShadowedByOtherClass' ]) asOrderedCollection detectMax: [ :entity |
+	shadower := (self model allFunctions select: [ :entity | entity name = 'shadowedName' ]) asOrderedCollection detectMax: [ :entity |
 		            entity sourceAnchor startPos ].
-	shadowed := (self model allClasses select: [ :entity | entity name = 'ClassShadowedByOtherClass' ]) asOrderedCollection detectMin: [ :entity |
+	shadowed := (self model allClasses select: [ :entity | entity name = 'shadowedName' ]) asOrderedCollection detectMin: [ :entity |
 		            entity sourceAnchor startPos ].
 
-	self assert: shadower name equals: 'ClassShadowedByOtherClass'.
+	self assert: shadower name equals: 'shadowedName'.
 	self deny: shadower isShadowed.
-	self assert: shadower typeContainer equals: (self moduleNamed: 'moduleWithShadowing').
+	self assert: shadower functionOwner equals: (self moduleNamed: 'moduleWithShadowing').
 	self assert: shadower shadowedEntity equals: shadowed.
-	self assert: shadower sourceText equals: ('class ClassShadowedByOtherClass:
-
-    def __init__(self):
-        self.i_var_of_shadowed_class = 3' copyReplaceAll: String cr with: String lf).
+	self assert: shadower sourceText equals: ('def shadowedName():
+    print("I''m the last one so I win")' copyReplaceAll: String cr with: String lf).
 
-	self assert: shadowed name equals: 'ClassShadowedByOtherClass'.
+	self assert: shadowed name equals: 'shadowedName'.
 	self assert: shadowed isShadowed.
 	self assert: shadowed typeContainer equals: (self moduleNamed: 'moduleWithShadowing').
 	self assert: shadowed shadowingEntity equals: shadower.
-	self assert: shadowed sourceText equals: ('class ClassShadowedByOtherClass:
-    c_var_of_shadowed_class = True' copyReplaceAll: String cr with: String lf)
+	self assert: shadowed sourceText equals: ('class shadowedName:
+    shadowedNameVar = 2' copyReplaceAll: String cr with: String lf)
 ]
 
 { #category : 'tests - attributes' }
@@ -559,6 +559,53 @@ FamixPythonProject1Test >> testFunctionShadowedByClass [
     return 2' copyReplaceAll: String cr with: String lf)
 ]
 
+{ #category : 'tests - shadowing' }
+FamixPythonProject1Test >> testFunctionShadowedByFunction [
+
+	| shadower shadowed |
+	shadower := (self model allFunctions select: [ :entity | entity name = 'function_to_shadow' ]) asOrderedCollection detectMax: [ :entity |
+		            entity sourceAnchor startPos ].
+	shadowed := (self model allFunctions select: [ :entity | entity name = 'function_to_shadow' ]) asOrderedCollection detectMin: [ :entity |
+		            entity sourceAnchor startPos ].
+
+	self assert: shadower name equals: 'function_to_shadow'.
+	self deny: shadower isShadowed.
+	self assert: shadower functionOwner equals: (self moduleNamed: 'moduleAtRoot6').
+	self assert: shadower shadowedEntity equals: shadowed.
+	self assert: shadower sourceText equals: ('def function_to_shadow():
+    return 2' copyReplaceAll: String cr with: String lf).
+
+	self assert: shadowed name equals: 'function_to_shadow'.
+	self assert: shadowed isShadowed.
+	self assert: shadowed functionOwner equals: (self moduleNamed: 'moduleAtRoot6').
+	self assert: shadowed shadowingEntity equals: shadower.
+	self assert: shadowed sourceText equals: ('def function_to_shadow():
+    return 1' copyReplaceAll: String cr with: String lf)
+]
+
+{ #category : 'tests - shadowing' }
+FamixPythonProject1Test >> testFunctionShadowedByGlobal [
+
+	| shadower shadowed |
+	shadower := (self model allGlobalVariables select: [ :entity | entity name = 'global_then_function_then_global_then_class' ]) asOrderedCollection detectMax: [ :entity |
+		            entity sourceAnchor startPos ].
+	shadowed := (self model allFunctions select: [ :entity | entity name = 'global_then_function_then_global_then_class' ]) asOrderedCollection detectMin: [ :entity |
+		            entity sourceAnchor startPos ].
+
+	self assert: shadower name equals: 'global_then_function_then_global_then_class'.
+	self assert: shadower isShadowed.  "<= In that case the shadower is also shadowed later"
+	self assert: shadower parentScope equals: (self moduleNamed: 'moduleWithShadowing').
+	self assert: shadower shadowedEntity equals: shadowed.
+	self assert: shadower sourceText equals: ('global_then_function_then_global_then_class' copyReplaceAll: String cr with: String lf).
+
+	self assert: shadowed name equals: 'global_then_function_then_global_then_class'.
+	self assert: shadowed isShadowed.
+	self assert: shadowed functionOwner equals: (self moduleNamed: 'moduleWithShadowing').
+	self assert: shadowed shadowingEntity equals: shadower.
+	self assert: shadowed sourceText equals: ('def global_then_function_then_global_then_class():
+    print(14)' copyReplaceAll: String cr with: String lf)
+]
+
 { #category : 'tests - functions' }
 FamixPythonProject1Test >> testFunctionSourceAnchor [
 
@@ -607,6 +654,29 @@ FamixPythonProject1Test >> testGetterAndSetter [
 	self deny: setter isConstructor
 ]
 
+{ #category : 'tests - shadowing' }
+FamixPythonProject1Test >> testGlobalShadowedByFunction [
+
+	| shadower shadowed |
+	shadower := (self model allFunctions select: [ :entity | entity name = 'global_then_function_then_global_then_class' ]) asOrderedCollection detectMin: [
+		            :entity | entity sourceAnchor startPos ].
+	shadowed := (self model allGlobalVariables select: [ :entity | entity name = 'global_then_function_then_global_then_class' ]) asOrderedCollection detectMin: [
+		            :entity | entity sourceAnchor startPos ].
+
+	self assert: shadower name equals: 'global_then_function_then_global_then_class'.
+	self assert: shadower isShadowed. "<= In that case the shadower is also shadowed later"
+	self assert: shadower functionOwner equals: (self moduleNamed: 'moduleWithShadowing').
+	self assert: shadower shadowedEntity equals: shadowed.
+	self assert: shadower sourceText equals: ('def global_then_function_then_global_then_class():
+    print(14)' copyReplaceAll: String cr with: String lf).
+
+	self assert: shadowed name equals: 'global_then_function_then_global_then_class'.
+	self assert: shadowed isShadowed.
+	self assert: shadowed parentScope equals: (self moduleNamed: 'moduleWithShadowing').
+	self assert: shadowed shadowingEntity equals: shadower.
+	self assert: shadowed sourceText equals: 'global_then_function_then_global_then_class'
+]
+
 { #category : 'tests - global variables' }
 FamixPythonProject1Test >> testGlobalVariableAreDefinedOnlyOnce [
 	"We should have only one global even if it is assigned multiple times."
@@ -1763,23 +1833,6 @@ FamixPythonProject1Test >> testRootPackage [
 	self assert: rootPackage isRoot
 ]
 
-{ #category : 'tests - shadowing' }
-FamixPythonProject1Test >> testShadowableFunction [
-
-	| function shadowable |
-	function := (self model allFunctions select: [ :funct | funct name = 'function_to_shadow' ]) asOrderedCollection detectMax: [ :funct |
-		            funct sourceAnchor startPos ].
-	shadowable := (self model allFunctions select: [ :funct | funct name = 'function_to_shadow' ]) asOrderedCollection detectMin: [ :funct |
-		              funct sourceAnchor startPos ].
-
-	self assert: function name equals: 'function_to_shadow'.
-	self deny: function isShadowed.
-	self assert: function functionOwner equals: (self moduleNamed: 'moduleAtRoot6').
-	self assert: function shadowedEntity equals: shadowable.
-	self assert: function sourceText equals: ('def function_to_shadow():
-    return 2' copyReplaceAll: String cr with: String lf)
-]
-
 { #category : 'tests - shadowing' }
 FamixPythonProject1Test >> testShadowableFunctionWithDifferentParameter [
 
@@ -1798,23 +1851,6 @@ FamixPythonProject1Test >> testShadowableFunctionWithDifferentParameter [
     return age' copyReplaceAll: String cr with: String lf)
 ]
 
-{ #category : 'tests - shadowing' }
-FamixPythonProject1Test >> testShadowedFunction [
-
-	| function shadower |
-	function := (self model allFunctions select: [ :funct | funct name = 'function_to_shadow' ]) asOrderedCollection detectMin: [ :funct |
-		            funct sourceAnchor startPos ].
-	shadower := (self model allFunctions select: [ :funct | funct name = 'function_to_shadow' ]) asOrderedCollection detectMax: [ :funct |
-		            funct sourceAnchor startPos ].
-
-	self assert: function name equals: 'function_to_shadow'.
-	self assert: function isShadowed.
-	self assert: function functionOwner equals: (self moduleNamed: 'moduleAtRoot6').
-	self assert: function shadowingEntity equals: shadower.
-	self assert: function sourceText equals: ('def function_to_shadow():
-    return 1' copyReplaceAll: String cr with: String lf)
-]
-
 { #category : 'tests - shadowing' }
 FamixPythonProject1Test >> testShadowedFunctionCreatesDifferentFunctions [
 

diff --git a/src/Famix-Python-Importer/FamixPythonImporterVisitor.class.st b/src/Famix-Python-Importer/FamixPythonImporterVisitor.class.st
@@ -298,14 +298,6 @@ FamixPythonImporterVisitor >> ensureStubPackagesFromPath: aPythonImportPath [
 	^ package
 ]
 
-{ #category : 'visiting' }
-FamixPythonImporterVisitor >> ensureVariable: aName localTo: aFamixEntity [
-
-	aFamixEntity dictLocalVariables
-		at: aName
-		ifAbsent: [ aFamixEntity createLocalVariable: aName ]
-]
-
 { #category : 'accessing' }
 FamixPythonImporterVisitor >> extractArgumentsInformation: aSignature [
 
@@ -841,13 +833,17 @@ FamixPythonImporterVisitor >> visitString: aStringNode [
 FamixPythonImporterVisitor >> visitVariableExpression: aVariableExpression [
 	"This node is used in multiple situations. If it is in an assignation we need to check if the variable we assign is created. Other uses can be for example in an import, in this case we want to only return the name."
 
-	isInLeftSideOfAssignation ifTrue: [
-		(self currentEntity childOfType: FamixTStructuralEntity named: aVariableExpression name) ifNil: [
-			| variable |
-			variable := self currentEntity createLocalVariable: aVariableExpression name.
-			"We select the lhs node because the source anchor should not be the full assignation."
-			self setSourceAnchor: variable from: aVariableExpression.
-			^ variable ] ].
+	isInLeftSideOfAssignation ifTrue: [ "If we have a variable of this name already, we should not recreate it. Except if this variable got shadowed! In that case, we should recreate it."
+		(self currentEntity query descendants ofType: FamixTStructuralEntity)
+			detect: [ :child | child name = aVariableExpression name and: [ child isShadowable not or: [ child isShadowed not ] ] ]
+			ifNone: [
+				| variable shadowedEntity |
+				shadowedEntity := self findSadowedEntityNamed: aVariableExpression name.
+				variable := self currentEntity createLocalVariable: aVariableExpression name.
+				shadowedEntity ifNotNil: [ variable shadowedEntity: shadowedEntity ].
+				"We select the lhs node because the source anchor should not be the full assignation."
+				self setSourceAnchor: variable from: aVariableExpression.
+				^ variable ] ].
 
 	^ aVariableExpression name
 ]