Python: Add suggested comments and test case

yoff · yoff · commit 913990bc6224 · 2021-09-03T14:40:16.000+02:00
diff --git a/python/ql/src/semmle/python/functions/ModificationOfParameterWithDefaultCustomizations.qll b/python/ql/src/semmle/python/functions/ModificationOfParameterWithDefaultCustomizations.qll
@@ -17,6 +17,7 @@ module ModificationOfParameterWithDefault {
    * A data flow source for detecting modifications of a parameters default value.
    */
   abstract class Source extends DataFlow::Node {
+    /** Result is true if the default value is non-empty for this source and false if not. */
     abstract boolean isNonEmpty();
   }
 
@@ -34,6 +35,7 @@ module ModificationOfParameterWithDefault {
    * A sanitizer guard for detecting modifications of a parameters default value.
    */
   abstract class BarrierGuard extends DataFlow::BarrierGuard {
+    /** Result is true if this guard blocks non-empty values and false if it blocks empty values. */
     abstract boolean blocksNonEmpty();
   }
 
@@ -120,7 +122,7 @@ module ModificationOfParameterWithDefault {
     }
   }
 
-  /** 
+  /**
    * An expression that is checked directly in an `if`, possibly with `not`, such as `if x:` or `if not x:`.
    */
   private class IdentityGuarded extends Expr {
@@ -139,7 +141,7 @@ module ModificationOfParameterWithDefault {
       )
     }
 
-    /** 
+    /**
      * Whether this guard has been inverted. For `if x:` the result is `false`, and for `if not x:` the result is `true`.
      */
     boolean isInverted() { result = inverted }
diff --git a/python/ql/test/query-tests/Functions/ModificationOfParameterWithDefault/test.py b/python/ql/test/query-tests/Functions/ModificationOfParameterWithDefault/test.py
@@ -124,5 +124,11 @@ def dict_update_op_nochange(d = {}):
 # OK
 def sanitizer(l = []):
     if l:
+        l.append(1)  #$ modification=l
+    return l
+
+# OK
+def sanitizer_negated(l = [1]):
+    if not l:
         l.append(1)  #$ SPURIOUS: modification=l
     return l

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ module ModificationOfParameterWithDefault {`
`17`	`17`	`* A data flow source for detecting modifications of a parameters default value.`
`18`	`18`	`*/`
`19`	`19`	`abstract class Source extends DataFlow::Node {`
	`20`	`+ /** Result is true if the default value is non-empty for this source and false if not. */`
`20`	`21`	`abstract boolean isNonEmpty();`
`21`	`22`	`}`
`22`	`23`
`@@ -34,6 +35,7 @@ module ModificationOfParameterWithDefault {`
`34`	`35`	`* A sanitizer guard for detecting modifications of a parameters default value.`
`35`	`36`	`*/`
`36`	`37`	`abstract class BarrierGuard extends DataFlow::BarrierGuard {`
	`38`	`+ /** Result is true if this guard blocks non-empty values and false if it blocks empty values. */`
`37`	`39`	`abstract boolean blocksNonEmpty();`
`38`	`40`	`}`
`39`	`41`
`@@ -120,7 +122,7 @@ module ModificationOfParameterWithDefault {`
`120`	`122`	`}`
`121`	`123`	`}`
`122`	`124`
`123`		`- /**`
	`125`	`+ /**`
`124`	`126`	* An expression that is checked directly in an `if`, possibly with `not`, such as `if x:` or `if not x:`.
`125`	`127`	`*/`
`126`	`128`	`private class IdentityGuarded extends Expr {`
`@@ -139,7 +141,7 @@ module ModificationOfParameterWithDefault {`
`139`	`141`	`)`
`140`	`142`	`}`
`141`	`143`
`142`		`- /**`
	`144`	`+ /**`
`143`	`145`	* Whether this guard has been inverted. For `if x:` the result is `false`, and for `if not x:` the result is `true`.
`144`	`146`	`*/`
`145`	`147`	`boolean isInverted() { result = inverted }`