mirror of
https://github.com/github/codeql.git
synced 2026-04-30 11:15:13 +02:00
Merge pull request #8366 from jketema/code-duplication-deprecated
C++: Mark everything in CodeDuplication.qll as deprecated
This commit is contained in:
@@ -426,7 +426,6 @@
|
||||
"python/ql/src/Lexical/CommentedOutCodeMetricOverview.inc.qhelp"
|
||||
],
|
||||
"FLinesOfDuplicatedCodeCommon.inc.qhelp": [
|
||||
"cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
|
||||
"java/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
|
||||
"javascript/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp",
|
||||
"python/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp"
|
||||
|
||||
@@ -1,6 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<include src="FLinesOfDuplicatedCodeCommon.inc.qhelp" />
|
||||
</qhelp>
|
||||
@@ -1,27 +0,0 @@
|
||||
/**
|
||||
* @deprecated
|
||||
* @name Duplicated lines in files
|
||||
* @description The number of lines in a file, including code, comment
|
||||
* and whitespace lines, which are duplicated in at least
|
||||
* one other place.
|
||||
* @kind treemap
|
||||
* @treemap.warnOn highValues
|
||||
* @metricType file
|
||||
* @metricAggregate avg sum max
|
||||
* @id cpp/duplicated-lines-in-files
|
||||
* @tags testability
|
||||
* modularity
|
||||
*/
|
||||
|
||||
import external.CodeDuplication
|
||||
|
||||
from File f, int n
|
||||
where
|
||||
n =
|
||||
count(int line |
|
||||
exists(DuplicateBlock d | d.sourceFile() = f |
|
||||
line in [d.sourceStartLine() .. d.sourceEndLine()]
|
||||
) and
|
||||
not whitelistedLineForDuplication(f, line)
|
||||
)
|
||||
select f, n order by n desc
|
||||
@@ -1,35 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
|
||||
<p>
|
||||
This metric measures the number of lines in a file that are contained within a block that is duplicated elsewhere. These lines may include code, comments and whitespace, and the duplicate block may be in this file or in another file.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
A file that contains many lines that are duplicated within the code base is problematic
|
||||
for a number of reasons.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<include src="DuplicationProblems.inc.qhelp" />
|
||||
|
||||
<recommendation>
|
||||
|
||||
<p>
|
||||
Refactor files with lots of duplicated code to extract the common code into
|
||||
a shared library or module.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
|
||||
<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Duplicate_code">Duplicate code</a>.</li>
|
||||
<li>M. Fowler, <em>Refactoring</em>. Addison-Wesley, 1999.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: deprecated
|
||||
---
|
||||
* The predicates and classes in the `CodeDuplication` library have been deprecated.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: breaking
|
||||
---
|
||||
* The deprecated queries `cpp/duplicate-block`, `cpp/duplicate-function`, `cpp/duplicate-class`, `cpp/duplicate-file`, `cpp/mostly-duplicate-function`,`cpp/similar-file`, `cpp/duplicated-lines-in-files` have been removed.
|
||||
114
cpp/ql/src/external/CodeDuplication.qll
vendored
114
cpp/ql/src/external/CodeDuplication.qll
vendored
@@ -2,14 +2,14 @@
|
||||
|
||||
import cpp
|
||||
|
||||
private newtype TDuplicationOrSimilarity = MKDuplicationOrSimilarity()
|
||||
deprecated private newtype TDuplicationOrSimilarity = MKDuplicationOrSimilarity()
|
||||
|
||||
/**
|
||||
* DEPRECATED: This class is no longer used.
|
||||
*
|
||||
* A token block used for detection of duplicate and similar code.
|
||||
*/
|
||||
class Copy extends TDuplicationOrSimilarity {
|
||||
deprecated class Copy extends TDuplicationOrSimilarity {
|
||||
/** Gets the index of the token in this block starting at the location `loc`, if any. */
|
||||
int tokenStartingAt(Location loc) { none() }
|
||||
|
||||
@@ -63,7 +63,7 @@ class Copy extends TDuplicationOrSimilarity {
|
||||
*
|
||||
* A block of duplicated code.
|
||||
*/
|
||||
class DuplicateBlock extends Copy {
|
||||
deprecated class DuplicateBlock extends Copy {
|
||||
override string toString() {
|
||||
result = "Duplicate code: " + this.sourceLines() + " duplicated lines."
|
||||
}
|
||||
@@ -74,21 +74,29 @@ class DuplicateBlock extends Copy {
|
||||
*
|
||||
* A block of similar code.
|
||||
*/
|
||||
class SimilarBlock extends Copy {
|
||||
deprecated class SimilarBlock extends Copy {
|
||||
override string toString() {
|
||||
result = "Similar code: " + this.sourceLines() + " almost duplicated lines."
|
||||
}
|
||||
}
|
||||
|
||||
/** Gets a function with a body and a location. */
|
||||
FunctionDeclarationEntry sourceMethod() {
|
||||
/**
|
||||
* DEPRECATED: The `CodeDuplication` library will be removed in a future release.
|
||||
*
|
||||
* Gets a function with a body and a location.
|
||||
*/
|
||||
deprecated FunctionDeclarationEntry sourceMethod() {
|
||||
result.isDefinition() and
|
||||
exists(result.getLocation()) and
|
||||
numlines(unresolveElement(result.getFunction()), _, _, _)
|
||||
}
|
||||
|
||||
/** Gets the number of member functions in `c` with a body and a location. */
|
||||
int numberOfSourceMethods(Class c) {
|
||||
/**
|
||||
* DEPRECATED: The `CodeDuplication` library will be removed in a future release.
|
||||
*
|
||||
* Gets the number of member functions in `c` with a body and a location.
|
||||
*/
|
||||
deprecated int numberOfSourceMethods(Class c) {
|
||||
result =
|
||||
count(FunctionDeclarationEntry m |
|
||||
m = sourceMethod() and
|
||||
@@ -96,7 +104,7 @@ int numberOfSourceMethods(Class c) {
|
||||
)
|
||||
}
|
||||
|
||||
private predicate blockCoversStatement(int equivClass, int first, int last, Stmt stmt) {
|
||||
deprecated private predicate blockCoversStatement(int equivClass, int first, int last, Stmt stmt) {
|
||||
exists(DuplicateBlock b, Location loc |
|
||||
stmt.getLocation() = loc and
|
||||
first = b.tokenStartingAt(loc) and
|
||||
@@ -105,13 +113,13 @@ private predicate blockCoversStatement(int equivClass, int first, int last, Stmt
|
||||
)
|
||||
}
|
||||
|
||||
private Stmt statementInMethod(FunctionDeclarationEntry m) {
|
||||
deprecated private Stmt statementInMethod(FunctionDeclarationEntry m) {
|
||||
result.getParent+() = m.getBlock() and
|
||||
not result.getLocation() instanceof UnknownStmtLocation and
|
||||
not result instanceof BlockStmt
|
||||
}
|
||||
|
||||
private predicate duplicateStatement(
|
||||
deprecated private predicate duplicateStatement(
|
||||
FunctionDeclarationEntry m1, FunctionDeclarationEntry m2, Stmt s1, Stmt s2
|
||||
) {
|
||||
exists(int equivClass, int first, int last |
|
||||
@@ -125,31 +133,39 @@ private predicate duplicateStatement(
|
||||
}
|
||||
|
||||
/**
|
||||
* DEPRECATED: Information on duplicated statements is no longer available.
|
||||
*
|
||||
* Holds if `m1` is a function with `total` lines, and `m2` is a function
|
||||
* that has `duplicate` lines in common with `m1`.
|
||||
*/
|
||||
predicate duplicateStatements(
|
||||
deprecated predicate duplicateStatements(
|
||||
FunctionDeclarationEntry m1, FunctionDeclarationEntry m2, int duplicate, int total
|
||||
) {
|
||||
duplicate = strictcount(Stmt s | duplicateStatement(m1, m2, s, _)) and
|
||||
total = strictcount(statementInMethod(m1))
|
||||
}
|
||||
|
||||
/** Holds if `m` and other are identical functions. */
|
||||
predicate duplicateMethod(FunctionDeclarationEntry m, FunctionDeclarationEntry other) {
|
||||
/**
|
||||
* DEPRECATED: Information on duplicated methods is no longer available.
|
||||
*
|
||||
* Holds if `m` and other are identical functions.
|
||||
*/
|
||||
deprecated predicate duplicateMethod(FunctionDeclarationEntry m, FunctionDeclarationEntry other) {
|
||||
exists(int total | duplicateStatements(m, other, total, total))
|
||||
}
|
||||
|
||||
/**
|
||||
* DEPRECATED: Information on similar lines is no longer available.
|
||||
*
|
||||
* INTERNAL: do not use.
|
||||
*
|
||||
* Holds if `line` in `f` is similar to a line somewhere else.
|
||||
*/
|
||||
predicate similarLines(File f, int line) {
|
||||
deprecated predicate similarLines(File f, int line) {
|
||||
exists(SimilarBlock b | b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()])
|
||||
}
|
||||
|
||||
private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, File f) {
|
||||
deprecated private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, File f) {
|
||||
lines =
|
||||
strictsum(SimilarBlock b, int toSum |
|
||||
(b.sourceFile() = f and b.getEquivalenceClass() = equivClass) and
|
||||
@@ -159,7 +175,7 @@ private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, Fil
|
||||
)
|
||||
}
|
||||
|
||||
private predicate similarLinesCoveredFiles(File f, File otherFile) {
|
||||
deprecated private predicate similarLinesCoveredFiles(File f, File otherFile) {
|
||||
exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
|
||||
exists(int coveredApprox |
|
||||
coveredApprox =
|
||||
@@ -175,8 +191,12 @@ private predicate similarLinesCoveredFiles(File f, File otherFile) {
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `coveredLines` lines of `f` are similar to lines in `otherFile`. */
|
||||
predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
|
||||
/**
|
||||
* DEPRECATED: Information on similar lines is no longer available.
|
||||
*
|
||||
* Holds if `coveredLines` lines of `f` are similar to lines in `otherFile`.
|
||||
*/
|
||||
deprecated predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
|
||||
exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
|
||||
similarLinesCoveredFiles(f, otherFile) and
|
||||
exists(int notCovered |
|
||||
@@ -191,17 +211,19 @@ predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
|
||||
}
|
||||
|
||||
/**
|
||||
* DEPRECATED: Information on duplicate lines is no longer available.
|
||||
*
|
||||
* INTERNAL: do not use.
|
||||
*
|
||||
* Holds if `line` in `f` is duplicated by a line somewhere else.
|
||||
*/
|
||||
predicate duplicateLines(File f, int line) {
|
||||
deprecated predicate duplicateLines(File f, int line) {
|
||||
exists(DuplicateBlock b |
|
||||
b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()]
|
||||
)
|
||||
}
|
||||
|
||||
private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, File f) {
|
||||
deprecated private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, File f) {
|
||||
lines =
|
||||
strictsum(DuplicateBlock b, int toSum |
|
||||
(b.sourceFile() = f and b.getEquivalenceClass() = equivClass) and
|
||||
@@ -211,8 +233,12 @@ private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, F
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `coveredLines` lines of `f` are duplicates of lines in `otherFile`. */
|
||||
predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
|
||||
/**
|
||||
* DEPRECATED: Information on duplicate lines is no longer available.
|
||||
*
|
||||
* Holds if `coveredLines` lines of `f` are duplicates of lines in `otherFile`.
|
||||
*/
|
||||
deprecated predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
|
||||
exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
|
||||
exists(int coveredApprox |
|
||||
coveredApprox =
|
||||
@@ -236,8 +262,12 @@ predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if most of `f` (`percent`%) is similar to `other`. */
|
||||
predicate similarFiles(File f, File other, int percent) {
|
||||
/**
|
||||
* DEPRECATED: Information on similar files is no longer available.
|
||||
*
|
||||
* Holds if most of `f` (`percent`%) is similar to `other`.
|
||||
*/
|
||||
deprecated predicate similarFiles(File f, File other, int percent) {
|
||||
exists(int covered, int total |
|
||||
similarLinesCovered(f, covered, other) and
|
||||
total = f.getMetrics().getNumberOfLines() and
|
||||
@@ -247,8 +277,12 @@ predicate similarFiles(File f, File other, int percent) {
|
||||
not duplicateFiles(f, other, _)
|
||||
}
|
||||
|
||||
/** Holds if most of `f` (`percent`%) is duplicated by `other`. */
|
||||
predicate duplicateFiles(File f, File other, int percent) {
|
||||
/**
|
||||
* DEPRECATED: Information on duplicate files is no longer available.
|
||||
*
|
||||
* Holds if most of `f` (`percent`%) is duplicated by `other`.
|
||||
*/
|
||||
deprecated predicate duplicateFiles(File f, File other, int percent) {
|
||||
exists(int covered, int total |
|
||||
duplicateLinesCovered(f, covered, other) and
|
||||
total = f.getMetrics().getNumberOfLines() and
|
||||
@@ -258,10 +292,12 @@ predicate duplicateFiles(File f, File other, int percent) {
|
||||
}
|
||||
|
||||
/**
|
||||
* DEPRECATED: Information on duplciate classes is no longer available.
|
||||
*
|
||||
* Holds if most member functions of `c` (`numDup` out of `total`) are
|
||||
* duplicates of member functions in `other`.
|
||||
*/
|
||||
predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total) {
|
||||
deprecated predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total) {
|
||||
numDup =
|
||||
strictcount(FunctionDeclarationEntry m1 |
|
||||
exists(FunctionDeclarationEntry m2 |
|
||||
@@ -277,11 +313,13 @@ predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total)
|
||||
}
|
||||
|
||||
/**
|
||||
* DEPRECATED: Information on duplciate classes is no longer available.
|
||||
*
|
||||
* Holds if most member functions of `c` are duplicates of member functions in
|
||||
* `other`. Provides the human-readable `message` to describe the amount of
|
||||
* duplication.
|
||||
*/
|
||||
predicate mostlyDuplicateClass(Class c, Class other, string message) {
|
||||
deprecated predicate mostlyDuplicateClass(Class c, Class other, string message) {
|
||||
exists(int numDup, int total |
|
||||
mostlyDuplicateClassBase(c, other, numDup, total) and
|
||||
(
|
||||
@@ -305,21 +343,31 @@ predicate mostlyDuplicateClass(Class c, Class other, string message) {
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `f` and `other` are similar or duplicates. */
|
||||
predicate fileLevelDuplication(File f, File other) {
|
||||
/**
|
||||
* DEPRECATED: Information on file duplication is no longer available.
|
||||
*
|
||||
* Holds if `f` and `other` are similar or duplicates.
|
||||
*/
|
||||
deprecated predicate fileLevelDuplication(File f, File other) {
|
||||
similarFiles(f, other, _) or duplicateFiles(f, other, _)
|
||||
}
|
||||
|
||||
/**
|
||||
* DEPRECATED: Information on class duplication is no longer available.
|
||||
*
|
||||
* Holds if most member functions of `c` are duplicates of member functions in
|
||||
* `other`.
|
||||
*/
|
||||
predicate classLevelDuplication(Class c, Class other) { mostlyDuplicateClass(c, other, _) }
|
||||
deprecated predicate classLevelDuplication(Class c, Class other) {
|
||||
mostlyDuplicateClass(c, other, _)
|
||||
}
|
||||
|
||||
/**
|
||||
* DEPRECATED: The CodeDuplication library will be removed in a future release.
|
||||
*
|
||||
* Holds if `line` in `f` should be allowed to be duplicated. This is the case
|
||||
* for `#include` directives.
|
||||
*/
|
||||
predicate whitelistedLineForDuplication(File f, int line) {
|
||||
deprecated predicate whitelistedLineForDuplication(File f, int line) {
|
||||
exists(Include i | i.getFile() = f and i.getLocation().getStartLine() = line)
|
||||
}
|
||||
|
||||
27
cpp/ql/src/external/DuplicateBlock.ql
vendored
27
cpp/ql/src/external/DuplicateBlock.ql
vendored
@@ -1,27 +0,0 @@
|
||||
/**
|
||||
* @deprecated
|
||||
* @name Duplicate code
|
||||
* @description This block of code is duplicated elsewhere. If possible, the shared code should be refactored so there is only one occurrence left. It may not always be possible to address these issues; other duplicate code checks (such as duplicate function, duplicate class) give subsets of the results with higher confidence.
|
||||
* @kind problem
|
||||
* @id cpp/duplicate-block
|
||||
* @problem.severity recommendation
|
||||
* @precision medium
|
||||
* @tags testability
|
||||
* maintainability
|
||||
* duplicate-code
|
||||
* non-attributable
|
||||
*/
|
||||
|
||||
import CodeDuplication
|
||||
|
||||
from DuplicateBlock d, DuplicateBlock other, int lines, File otherFile, int otherLine
|
||||
where
|
||||
lines = d.sourceLines() and
|
||||
lines > 10 and
|
||||
other.getEquivalenceClass() = d.getEquivalenceClass() and
|
||||
other != d and
|
||||
otherFile = other.sourceFile() and
|
||||
otherLine = other.sourceStartLine()
|
||||
select d,
|
||||
"Duplicate code: " + lines + " lines are duplicated at " + otherFile.getBaseName() + ":" +
|
||||
otherLine
|
||||
43
cpp/ql/src/external/DuplicateFunction.qhelp
vendored
43
cpp/ql/src/external/DuplicateFunction.qhelp
vendored
@@ -1,43 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>A function should never be duplicated verbatim in several places in the code. Of course
|
||||
the severity of this anti-pattern is higher for longer functions than for extremely short
|
||||
functions of one or two statements, but there are usually better ways of achieving the same
|
||||
effect.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
|
||||
inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
|
||||
can mask the real purpose or intention behind a function. There is also a risk of
|
||||
update anomalies, where only one of several copies of the code is updated to address a defect or
|
||||
add a feature.</p>
|
||||
|
||||
<p>In the case of function duplication, how to address the issue depends on the functions themselves
|
||||
and on the precise classes in which the duplication occurs. At its simplest, the duplication can
|
||||
be addressed by removing all but one of the duplicate function definitions and making
|
||||
callers of the removed functions refer to the (now canonical) single remaining definition
|
||||
instead.</p>
|
||||
|
||||
<p>This may not be possible for reasons of visibility or accessibility. A common example might
|
||||
be where two classes implement the same functionality but neither is a subtype of the other,
|
||||
so it is not possible to inherit a single function definition. In such cases, introducing a
|
||||
common superclass to share the duplicated code is a viable option. Alternatively, if the functions
|
||||
don't need access to private object state, they can be moved to a shared utility class that
|
||||
just provides the functionality itself.</p>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009.
|
||||
Do code clones matter? In <em>Proceedings of the 31st International Conference on
|
||||
Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA,
|
||||
485-495.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
39
cpp/ql/src/external/DuplicateFunction.ql
vendored
39
cpp/ql/src/external/DuplicateFunction.ql
vendored
@@ -1,39 +0,0 @@
|
||||
/**
|
||||
* @deprecated
|
||||
* @name Duplicate function
|
||||
* @description There is another identical implementation of this function. Extract the code to a common file or superclass or delegate to improve sharing.
|
||||
* @kind problem
|
||||
* @id cpp/duplicate-function
|
||||
* @problem.severity recommendation
|
||||
* @precision medium
|
||||
* @tags testability
|
||||
* maintainability
|
||||
* duplicate-code
|
||||
* non-attributable
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import CodeDuplication
|
||||
|
||||
predicate relevant(FunctionDeclarationEntry m) {
|
||||
exists(Location loc |
|
||||
loc = m.getBlock().getLocation() and
|
||||
(
|
||||
loc.getStartLine() + 5 < loc.getEndLine() and not m.getName().matches("get%")
|
||||
or
|
||||
loc.getStartLine() + 10 < loc.getEndLine()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
from FunctionDeclarationEntry m, FunctionDeclarationEntry other
|
||||
where
|
||||
duplicateMethod(m, other) and
|
||||
relevant(m) and
|
||||
not m.getFunction().isConstructedFrom(_) and
|
||||
not other.getFunction().isConstructedFrom(_) and
|
||||
not fileLevelDuplication(m.getFile(), other.getFile()) and
|
||||
not classLevelDuplication(m.getFunction().getDeclaringType(),
|
||||
other.getFunction().getDeclaringType())
|
||||
select m, "Function " + m.getName() + " is duplicated at $@.", other,
|
||||
other.getFile().getBaseName() + ":" + other.getLocation().getStartLine().toString()
|
||||
45
cpp/ql/src/external/MostlyDuplicateClass.qhelp
vendored
45
cpp/ql/src/external/MostlyDuplicateClass.qhelp
vendored
@@ -1,45 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>In cases where several methods are duplicated between two (or more) classes,
|
||||
the classes themselves are highlighted as "mostly duplicate", rather than creating a large
|
||||
number of method-level warnings. The same caveats apply here, too.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
|
||||
inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
|
||||
can mask the real purpose or intention behind a method. There's also an omnipresent risk of
|
||||
update anomalies, where only one of several copies of the code is updated to address a defect or
|
||||
add a feature.</p>
|
||||
|
||||
<p>While completely duplicated classes are rare, they are usually a sign of a simple
|
||||
oversight (or deliberate copy/paste) on a developer's part. Usually the required remedy
|
||||
action is to remove all but one of them.</p>
|
||||
|
||||
<p>It is far more common to see duplication of many methods between two classes, leaving just
|
||||
a few that are actually different. Such situations warrant close inspection. Are the differences
|
||||
deliberate or a result of an inconsistent update to one of the clones? If the latter, then
|
||||
treating the classes as completely duplicate and eliminating all but one (while preserving any
|
||||
corrections or new features that may have been introduced) is the best course. If two classes serve
|
||||
genuinely different purposes but almost all of their methods are the same, that can be a sign
|
||||
that there is a missing level of abstraction. Introducing a common superclass to define the
|
||||
common methods and sharing the code is likely to prevent many problems in the long term.</p>
|
||||
|
||||
<p>Modern IDEs may provide refactoring support for this sort of transformation, usually
|
||||
under the names of "Pull up" or "Extract supertype".</p>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009.
|
||||
Do code clones matter? In <em>Proceedings of the 31st International Conference on
|
||||
Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA,
|
||||
485-495.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
24
cpp/ql/src/external/MostlyDuplicateClass.ql
vendored
24
cpp/ql/src/external/MostlyDuplicateClass.ql
vendored
@@ -1,24 +0,0 @@
|
||||
/**
|
||||
* @deprecated
|
||||
* @name Mostly duplicate class
|
||||
* @description More than 80% of the methods in this class are duplicated in another class. Create a common supertype to improve code sharing.
|
||||
* @kind problem
|
||||
* @id cpp/duplicate-class
|
||||
* @problem.severity recommendation
|
||||
* @precision medium
|
||||
* @tags testability
|
||||
* maintainability
|
||||
* duplicate-code
|
||||
* non-attributable
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import CodeDuplication
|
||||
|
||||
from Class c, Class other, string message
|
||||
where
|
||||
mostlyDuplicateClass(c, other, message) and
|
||||
not c.isConstructedFrom(_) and
|
||||
not other.isConstructedFrom(_) and
|
||||
not fileLevelDuplication(c.getFile(), _)
|
||||
select c, message, other, other.getQualifiedName()
|
||||
46
cpp/ql/src/external/MostlyDuplicateFile.qhelp
vendored
46
cpp/ql/src/external/MostlyDuplicateFile.qhelp
vendored
@@ -1,46 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>In cases where most of a file's lines have been duplicated in one or more other
|
||||
files, the files themselves are highlighted as "mostly duplicate", rather than
|
||||
creating a large number of method-level or class-level warnings. The same caveats
|
||||
apply here, too.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
|
||||
inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
|
||||
can mask the real purpose or intention behind a method. There's also an omnipresent risk of
|
||||
update anomalies, where only one of several copies of the code is updated to address a defect or
|
||||
add a feature.</p>
|
||||
|
||||
<p>While completely duplicated files are rare, they are usually a sign of a simple
|
||||
oversight (or deliberate copy/paste) on a developer's part. Usually the required remedy
|
||||
action is to remove all but one of them. A common exception may arise from generated code
|
||||
that simply occurs in several places in the source tree; the check can be adapted to
|
||||
exclude such results.</p>
|
||||
|
||||
<p>It is far more common to see duplication of many lines between two files, leaving just
|
||||
a few that are actually different. Such situations warrant close inspection. Are the differences
|
||||
deliberate or a result of an inconsistent update to one of the clones? If the latter, then
|
||||
treating the files as completely duplicate and eliminating all but one (while preserving any
|
||||
corrections or new features that may have been introduced) is the best course. If two files serve
|
||||
genuinely different purposes but almost all of their lines are the same, that can be a sign
|
||||
that there is a missing level of abstraction. Look for ways to share the functionality, either
|
||||
by extracting a utility class for parts of it or by encapsulating the common parts into a new
|
||||
super class of any classes involved.</p>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009.
|
||||
Do code clones matter? In <em>Proceedings of the 31st International Conference on
|
||||
Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA,
|
||||
485-495.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
21
cpp/ql/src/external/MostlyDuplicateFile.ql
vendored
21
cpp/ql/src/external/MostlyDuplicateFile.ql
vendored
@@ -1,21 +0,0 @@
|
||||
/**
|
||||
* @deprecated
|
||||
* @name Mostly duplicate file
|
||||
* @description There is another file that shares a lot of the code with this file. Merge the two files to improve maintainability.
|
||||
* @kind problem
|
||||
* @id cpp/duplicate-file
|
||||
* @problem.severity recommendation
|
||||
* @precision medium
|
||||
* @tags testability
|
||||
* maintainability
|
||||
* duplicate-code
|
||||
* non-attributable
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import CodeDuplication
|
||||
|
||||
from File f, File other, int percent
|
||||
where duplicateFiles(f, other, percent)
|
||||
select f, percent + "% of the lines in " + f.getBaseName() + " are copies of lines in $@.", other,
|
||||
other.getBaseName()
|
||||
@@ -1,55 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>A "mostly duplicate function" is a function for which there is at least one
|
||||
almost exact duplicate somewhere else in the code, but for which there are no
|
||||
exact duplicates. There will be minor typographical differences between this
|
||||
function and any "mostly duplicate function" to which it corresponds (for
|
||||
example, comments and small code changes), preventing an exact match. Pairs of
|
||||
such functions are sometimes referred to as "similar".</p>
|
||||
|
||||
<p>This class of problem can often be more insidious than mere duplication, because the two
|
||||
implementations have diverged. This may be on purpose (when a function is copy-and-pasted
|
||||
and adapted to a new context) or accidentally (when a correction is only introduced in one of
|
||||
several identical pieces of code), and to address the problem one needs to understand which
|
||||
of the two situations applies.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
|
||||
inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
|
||||
can mask the real purpose or intention behind a function. There's also a risk of
|
||||
update anomalies, where only one of several copies of the code is updated to address a defect or
|
||||
add a feature.</p>
|
||||
|
||||
<p>In the case of function similarity, how to address the issue depends on the functions
|
||||
themselves and on the precise classes in which they occur. At its simplest, if the differences
|
||||
are accidental, the problem can be addressed by unifying the functions to behave identically.
|
||||
Then, we can remove all but one of the duplicate function definitions and make
|
||||
callers of the removed functions refer to the (now canonical) single remaining definition
|
||||
instead.</p>
|
||||
|
||||
<p>In more complex cases, look for ways of encapsulating the commonality and sharing it while
|
||||
retaining the differences in functionality. Perhaps the function can be moved to a single place
|
||||
and given an additional parameter, allowing it to cover all use cases? Alternatively, there
|
||||
may be a common preprocessing or postprocessing step which can be extracted to its own (shared)
|
||||
function, leaving only the specific parts in the existing functions.</p>
|
||||
|
||||
<p>Modern IDEs may provide refactoring support for this sort of transformation. Relevant
|
||||
refactorings might be "Extract function", "Change function signature", "Pull up" or "Extract
|
||||
supertype".</p>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009.
|
||||
Do code clones matter? In <em>Proceedings of the 31st International Conference on
|
||||
Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA,
|
||||
485-495.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
31
cpp/ql/src/external/MostlyDuplicateFunction.ql
vendored
31
cpp/ql/src/external/MostlyDuplicateFunction.ql
vendored
@@ -1,31 +0,0 @@
|
||||
/**
|
||||
* @deprecated
|
||||
* @name Mostly duplicate function
|
||||
* @description There is another function that shares a lot of the code with this one. Extract the code to a common file/superclass or delegate to improve sharing.
|
||||
* @kind problem
|
||||
* @id cpp/mostly-duplicate-function
|
||||
* @problem.severity recommendation
|
||||
* @precision medium
|
||||
* @tags testability
|
||||
* duplicate-code
|
||||
* non-attributable
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import CodeDuplication
|
||||
|
||||
from FunctionDeclarationEntry m, int covered, int total, FunctionDeclarationEntry other, int percent
|
||||
where
|
||||
duplicateStatements(m, other, covered, total) and
|
||||
covered != total and
|
||||
total > 5 and
|
||||
covered * 100 / total = percent and
|
||||
percent > 80 and
|
||||
not m.getFunction().isConstructedFrom(_) and
|
||||
not other.getFunction().isConstructedFrom(_) and
|
||||
not duplicateMethod(m, other) and
|
||||
not classLevelDuplication(m.getFunction().getDeclaringType(),
|
||||
other.getFunction().getDeclaringType()) and
|
||||
not fileLevelDuplication(m.getFile(), other.getFile())
|
||||
select m, percent + "% of the statements in " + m.getName() + " are duplicated in $@.", other,
|
||||
other.getFunction().getQualifiedName()
|
||||
39
cpp/ql/src/external/MostlySimilarFile.qhelp
vendored
39
cpp/ql/src/external/MostlySimilarFile.qhelp
vendored
@@ -1,39 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>Two lines are defined as similar if they are either identical or contain only very minor
|
||||
differences. In cases where most of the lines in a file have corresponding "similar" lines in another file,
|
||||
the files themselves are highlighted as "mostly similar", instead of creating a large number of method-level or class-level
|
||||
warnings. The same caveats apply here, too.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
|
||||
inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
|
||||
can mask the real purpose or intention behind a method. There's also an omnipresent risk of
|
||||
update anomalies, where only one of several copies of the code is updated to address a defect or
|
||||
add a feature.</p>
|
||||
|
||||
<p>With files that are marked as mostly similar, special care should be taken. Why are
|
||||
they almost the same, and why are there differences? If the differences are accidental (for
|
||||
example from corrections that were only applied to one copy), then unifying the files and removing
|
||||
all but one is the best thing to do. If the files have genuinely different tasks, it is worth
|
||||
thinking about the reasons for the similarity. Can some of the shared code be extracted into
|
||||
methods (perhaps with additional parameters, to cover the differences in behavior)? Should it
|
||||
be moved into a utility class or file that is accessible to all current implementations, or
|
||||
should a new level of abstraction be introduced?</p>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009.
|
||||
Do code clones matter? In <em>Proceedings of the 31st International Conference on
|
||||
Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA,
|
||||
485-495.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
21
cpp/ql/src/external/MostlySimilarFile.ql
vendored
21
cpp/ql/src/external/MostlySimilarFile.ql
vendored
@@ -1,21 +0,0 @@
|
||||
/**
|
||||
* @deprecated
|
||||
* @name Mostly similar file
|
||||
* @description There is another file that shares a lot of the code with this file. Notice that names of variables and types may have been changed. Merge the two files to improve maintainability.
|
||||
* @kind problem
|
||||
* @id cpp/similar-file
|
||||
* @problem.severity recommendation
|
||||
* @precision medium
|
||||
* @tags testability
|
||||
* maintainability
|
||||
* duplicate-code
|
||||
* non-attributable
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import CodeDuplication
|
||||
|
||||
from File f, File other, int percent
|
||||
where similarFiles(f, other, percent)
|
||||
select f, percent + "% of the lines in " + f.getBaseName() + " are similar to lines in $@.", other,
|
||||
other.getBaseName()
|
||||
Reference in New Issue
Block a user