mirror of
https://github.com/github/codeql.git
synced 2026-02-10 04:01:11 +01:00
Remove some FPs.
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
/**
|
||||
* @name Bidirectional imports for abstract classes
|
||||
* @description An abstract class should import each of its subclasses, unless it is meant as an extension point, in which case it should import none of them.
|
||||
* @description An abstract class should import each of its subclasses, unless
|
||||
* it is meant as a configuration-style class, in which case it
|
||||
* should import none of them.
|
||||
* @kind problem
|
||||
* @problem.severity error
|
||||
* @id ql/abstract-class-import
|
||||
@@ -19,12 +21,31 @@ File imports(File file) {
|
||||
)
|
||||
}
|
||||
|
||||
/** Gets a non-abstract subclass of `ab` that is defined in a different file */
|
||||
predicate testFile(File f) { f.getRelativePath().matches("%/ql/test/%") }
|
||||
|
||||
predicate nonTestQuery(File f) { f.getBaseName().matches("%.ql") and not testFile(f) }
|
||||
|
||||
predicate liveNonTestFile(File f) {
|
||||
exists(File query | nonTestQuery(query) and f = imports*(query))
|
||||
}
|
||||
|
||||
predicate experimentalFile(File f) { f.getRelativePath().matches("%/experimental/%") }
|
||||
|
||||
Class getASubclassOfAbstract(Class ab) {
|
||||
ab.isAbstract() and
|
||||
result.getType().getASuperType() = ab.getType()
|
||||
}
|
||||
|
||||
/** Gets a non-abstract subclass of `ab` that contributes to the extent of `ab`. */
|
||||
Class concreteExternalSubclass(Class ab) {
|
||||
ab.isAbstract() and
|
||||
not result.isAbstract() and
|
||||
result.getType().getASuperType*() = ab.getType() and
|
||||
result.getLocation().getFile() != ab.getLocation().getFile()
|
||||
result = getASubclassOfAbstract+(ab) and
|
||||
// Heuristic: An abstract class with subclasses in the same file and no other
|
||||
// imported subclasses is likely intentional.
|
||||
result.getLocation().getFile() != ab.getLocation().getFile() and
|
||||
// Exclude subclasses in tests and libraries that are only used in tests.
|
||||
liveNonTestFile(result.getLocation().getFile())
|
||||
}
|
||||
|
||||
/** Holds if there is a bidirectional import between the abstract class `ab` and its subclass `sub` */
|
||||
@@ -74,7 +95,9 @@ predicate alert(Class ab, string msg, Class sub, Class sub2) {
|
||||
"This abstract class imports its subclass $@ but doesn't import " + nonImports +
|
||||
" other subclasses, such as $@."
|
||||
)
|
||||
)
|
||||
) and
|
||||
// exclude results in experimental
|
||||
not experimentalFile(sub.getLocation().getFile())
|
||||
}
|
||||
|
||||
from Class ab, string msg, Class sub, Class sub2
|
||||
|
||||
Reference in New Issue
Block a user