SONARPY-498 Fix False Positive S4143: assigned collection in rhs (SonarSource#559)

vilchik-elena · web-flow · commit d23b808351a5 · 2020-02-10T19:02:17.000+01:00
diff --git a/its/ruling/src/test/resources/expected/python-S4143.json b/its/ruling/src/test/resources/expected/python-S4143.json
@@ -30,7 +30,4 @@
 'project:twisted-12.1.0/twisted/test/test_dirdbm.py':[
 26,
 ],
-'project:twisted-12.1.0/twisted/words/protocols/irc.py':[
-2910,
-],
 }
diff --git a/python-checks/src/main/java/org/sonar/python/checks/OverwrittenCollectionEntryCheck.java b/python-checks/src/main/java/org/sonar/python/checks/OverwrittenCollectionEntryCheck.java
@@ -106,15 +106,15 @@ private static CollectionWrite collectionWrite(AssignmentStatement assignment, E
     if (collection.is(Kind.SLICE_EXPR, Kind.SUBSCRIPTION)) {
       CollectionWrite nested = collectionWrite(assignment, collection);
       if (nested != null) {
-        return new CollectionWrite(nested.collectionKey.nest(key), nested.leftBracket, rBracket, assignment);
+        return new CollectionWrite(nested.collectionKey.nest(key), nested.leftBracket, rBracket, assignment, collection);
       }
     }
 
     if (collection instanceof HasSymbol) {
       Symbol symbol = ((HasSymbol) collection).symbol();
       if (symbol != null) {
         CollectionKey collectionKey = new CollectionKey(symbol, key);
-        return new CollectionWrite(collectionKey, lBracket, rBracket, assignment);
+        return new CollectionWrite(collectionKey, lBracket, rBracket, assignment, collection);
       }
     }
 
@@ -163,8 +163,7 @@ private static void reportOverwrites(SubscriptionContext ctx, Map<CollectionKey,
         CollectionWrite firstWrite = writes.get(0);
         CollectionWrite secondWrite = writes.get(1);
         AssignmentStatement assignment = secondWrite.assignment;
-        Expression lhs = lhs(assignment);
-        if (TreeUtils.hasDescendant(assignment.assignedValue(), t -> CheckUtils.areEquivalent(lhs, t))) {
+        if (TreeUtils.hasDescendant(assignment.assignedValue(), t -> CheckUtils.areEquivalent(firstWrite.collection, t))) {
           return;
         }
         String message = String.format(
@@ -192,12 +191,14 @@ private static class CollectionWrite {
     private final Token leftBracket;
     private final Token rightBracket;
     private final AssignmentStatement assignment;
+    private final Expression collection;
 
-    private CollectionWrite(CollectionKey collectionKey, Token leftBracket, Token rightBracket, AssignmentStatement assignment) {
+    private CollectionWrite(CollectionKey collectionKey, Token leftBracket, Token rightBracket, AssignmentStatement assignment, Expression collection) {
       this.collectionKey = collectionKey;
       this.leftBracket = leftBracket;
       this.rightBracket = rightBracket;
       this.assignment = assignment;
+      this.collection = collection;
     }
   }
 }
diff --git a/python-checks/src/test/resources/checks/overwrittenCollectionEntryCheck.py b/python-checks/src/test/resources/checks/overwrittenCollectionEntryCheck.py
@@ -91,3 +91,13 @@ def non_trivial_collections(f, obj1, obj2):
 def valid_replacement(list1, foo, x):
     list1["foo"] = foo(x)
     list1["foo"] = list1["foo"].replace("a", "b")
+
+def used_collection(list1):
+    list1[1] = 42
+    list1[1] = foo(list1[1])
+
+    list1[2] = 42
+    list1[2] = foo(list1)
+
+    list[3] = 42
+    list[3] = list[1] # FN

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,4 @@`
`30`	`30`	`'project:twisted-12.1.0/twisted/test/test_dirdbm.py':[`
`31`	`31`	`26,`
`32`	`32`	`],`
`33`		`-'project:twisted-12.1.0/twisted/words/protocols/irc.py':[`
`34`		`-2910,`
`35`		`-],`
`36`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -106,15 +106,15 @@ private static CollectionWrite collectionWrite(AssignmentStatement assignment, E`
`106`	`106`	`if (collection.is(Kind.SLICE_EXPR, Kind.SUBSCRIPTION)) {`
`107`	`107`	`CollectionWrite nested = collectionWrite(assignment, collection);`
`108`	`108`	`if (nested != null) {`
`109`		`- return new CollectionWrite(nested.collectionKey.nest(key), nested.leftBracket, rBracket, assignment);`
	`109`	`+ return new CollectionWrite(nested.collectionKey.nest(key), nested.leftBracket, rBracket, assignment, collection);`
`110`	`110`	`}`
`111`	`111`	`}`
`112`	`112`
`113`	`113`	`if (collection instanceof HasSymbol) {`
`114`	`114`	`Symbol symbol = ((HasSymbol) collection).symbol();`
`115`	`115`	`if (symbol != null) {`
`116`	`116`	`CollectionKey collectionKey = new CollectionKey(symbol, key);`
`117`		`- return new CollectionWrite(collectionKey, lBracket, rBracket, assignment);`
	`117`	`+ return new CollectionWrite(collectionKey, lBracket, rBracket, assignment, collection);`
`118`	`118`	`}`
`119`	`119`	`}`
`120`	`120`
`@@ -163,8 +163,7 @@ private static void reportOverwrites(SubscriptionContext ctx, Map<CollectionKey,`
`163`	`163`	`CollectionWrite firstWrite = writes.get(0);`
`164`	`164`	`CollectionWrite secondWrite = writes.get(1);`
`165`	`165`	`AssignmentStatement assignment = secondWrite.assignment;`
`166`		`- Expression lhs = lhs(assignment);`
`167`		`- if (TreeUtils.hasDescendant(assignment.assignedValue(), t -> CheckUtils.areEquivalent(lhs, t))) {`
	`166`	`+ if (TreeUtils.hasDescendant(assignment.assignedValue(), t -> CheckUtils.areEquivalent(firstWrite.collection, t))) {`
`168`	`167`	`return;`
`169`	`168`	`}`
`170`	`169`	`String message = String.format(`
`@@ -192,12 +191,14 @@ private static class CollectionWrite {`
`192`	`191`	`private final Token leftBracket;`
`193`	`192`	`private final Token rightBracket;`
`194`	`193`	`private final AssignmentStatement assignment;`
	`194`	`+ private final Expression collection;`
`195`	`195`
`196`		`- private CollectionWrite(CollectionKey collectionKey, Token leftBracket, Token rightBracket, AssignmentStatement assignment) {`
	`196`	`+ private CollectionWrite(CollectionKey collectionKey, Token leftBracket, Token rightBracket, AssignmentStatement assignment, Expression collection) {`
`197`	`197`	`this.collectionKey = collectionKey;`
`198`	`198`	`this.leftBracket = leftBracket;`
`199`	`199`	`this.rightBracket = rightBracket;`
`200`	`200`	`this.assignment = assignment;`
	`201`	`+ this.collection = collection;`
`201`	`202`	`}`
`202`	`203`	`}`
`203`	`204`	`}`