Merge pull request #484 from geoffw0/limitedscopefile

CPP: Fix Limitedscopefile.ql

Author: jbj

change-notes/1.19/analysis-cpp.md

@@ -17,6 +17,7 @@
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Empty branch of conditional | Fewer false positive results | The query now recognizes commented blocks more reliably. |
 | Expression has no effect | Fewer false positive results | Expressions in template instantiations are now excluded from this query. |
+| Global could be static | Fewer false positive results | Variables with declarations in header files are now excluded from this query. |
 | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. Also fixed an issue where false positives could occur if the destructor body was not in the snapshot. |
 | Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. |
 | Missing return statement | Fewer false positive results | The query is now produces correct results when a function returns a template-dependent type. |

cpp/ql/src/JPL_C/LOC-3/Rule 13/LimitedScopeFile.ql

@@ -12,5 +12,6 @@ from GlobalVariable v
 where forex(VariableAccess va | va.getTarget() = v | va.getFile() = v.getDefinitionLocation().getFile())
       and not v.hasSpecifier("static")
       and strictcount(v.getAnAccess().getEnclosingFunction()) > 1 // If = 1, variable should be function-scope.
+      and not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
 select v, "The global variable " + v.getName() + " is not accessed outside of " + v.getFile().getBaseName()
       + " and could be made static."

cpp/ql/src/Power of 10/Rule 6/GlobalCouldBeStatic.ql

@@ -12,5 +12,6 @@ from GlobalVariable v
 where forex(VariableAccess va | va.getTarget() = v | va.getFile() = v.getDefinitionLocation().getFile())
       and not v.hasSpecifier("static")
       and strictcount(v.getAnAccess().getEnclosingFunction()) > 1 // If = 1, variable should be function-scope.
+      and not v.getADeclarationEntry().getFile() instanceof HeaderFile // intended to be accessed elsewhere
 select v, "The global variable " + v.getName() + " is not accessed outside of " + v.getFile().getBaseName() +
       " and could be made static."

cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 13/LimitedScopeFile/LimitedScopeFile.expected

@@ -0,0 +1 @@
+| file1.c:3:5:3:14 | globalInt1 | The global variable globalInt1 is not accessed outside of file1.c and could be made static. |

cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 13/LimitedScopeFile/LimitedScopeFile.qlref

@@ -0,0 +1 @@
+JPL_C/LOC-3/Rule 13/LimitedScopeFile.ql

cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 13/LimitedScopeFile/file1.c

@@ -0,0 +1,25 @@
+// file1.c
+
+int globalInt1; // BAD [only accessed in this file]
+int globalInt2; // GOOD [accessed in file1.c and file2.c]
+int globalInt3; // GOOD [referenced in file1.h]
+int globalInt4; // GOOD [only accessed in one function, should be function scope instead]
+int globalInt5; // GOOD [not accessed]
+
+static int staticInt1; // GOOD [static]
+
+void file1Func1()
+{
+	globalInt1++;
+	globalInt2++;
+	globalInt3++;
+	globalInt4++;
+	staticInt1++;
+}
+
+void file1Func2()
+{
+	globalInt1++;
+	globalInt3++;
+	staticInt1++;
+}

cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 13/LimitedScopeFile/file1.h

@@ -0,0 +1,3 @@
+// file1.h
+
+extern int globalInt3;

cpp/ql/test/query-tests/JPL_C/LOC-3/Rule 13/LimitedScopeFile/file2.c

@@ -0,0 +1,10 @@
+// file2.c
+
+#include "file1.h"
+
+extern int globalInt2;
+
+void file2Func()
+{
+	globalInt2++;
+}