-
Notifications
You must be signed in to change notification settings - Fork 66
Add support for deviations on next line and multiple lines #807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
f3a6c3f
Deviations: remove no longer required query
lcartey 7b2a2e0
Add library for more complex code identifier deviations
lcartey f173072
Deviations: Integrate extended code-identifier deviations
lcartey e36828d
Deviations: Add test cases for new code-identifier deviations
lcartey e077935
Update user manual for new code identifier deviations
lcartey 6e68fb8
Deviations: Support an attribute like comment syntax
lcartey 1a24541
Deviations: Support C/C++ attributes
lcartey 4283652
Deviations: Switch to new deviations format
lcartey c65f635
Remove deviation suppression query tests
lcartey f56fa0f
Detect invalid deviation markers
lcartey bf62d9a
Merge branch 'main' into lcartey/extend-deviations
lcartey 5a6d7ca
Deviations: Support attribute inheritence
lcartey e38e416
Reformatting test file
lcartey 9f35565
Add change note, update manual.
lcartey d2f8670
Reformat file
lcartey 0de32d3
Update expected results
lcartey 029537d
Fix typos
lcartey 3f1997b
Deviations: Highlight invalid starts
lcartey 665a7d1
Improve documentation
lcartey 8437c7c
Deviation: Refactor begin/end for clarity.
lcartey 20669c7
Deviation: Clarification in user manual.
lcartey 9e35e59
Deviations: use getADeviationRecord
lcartey b273d0f
Deviations: Update expected tests
lcartey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
- A new in code deviation format has been introduced, using the C/C++ attribute syntax: | ||
``` | ||
[[codeql::<standard>_deviation("<code-identifier>")]] | ||
``` | ||
This can be applied to functions, statements and variables to apply a deviation from the Coding Standards configuration file. The user manual has been updated to describe the new format. | ||
- For those codebases that cannot use standard attributes, we have also introduced a comment based syntax | ||
``` | ||
// codeql::<standard>_deviation(<code-identifier>) | ||
// codeql::<standard>_deviation_next_line(<code-identifier>) | ||
// codeql::<standard>_deviation_begin(<code-identifier>) | ||
// codeql::<standard>_deviation_end(<code-identifier>) | ||
``` | ||
Further information is available in the user manual. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
349 changes: 349 additions & 0 deletions
349
cpp/common/src/codingstandards/cpp/deviations/CodeIdentifierDeviation.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,349 @@ | ||
/** | ||
* A module for identifying in code markers in code that trigger deviations. | ||
* | ||
* This module supports two different code identifier markers: | ||
* - A C/C++ attribute based syntax | ||
* - A comment-based format | ||
* | ||
* The C/C++ attribute based syntax uses the following format: | ||
* ``` | ||
* [[codeql::<standard>_deviation("code-identifier")]] | ||
* ``` | ||
* The deviation will be applied to the selected program element, and any syntactically nested children of that program element. | ||
* | ||
* For the comment format the marker consists of a `code-identifier` with some optional annotations. A deviation will be applied to | ||
* some range of lines in the file containing the comment based on the annotation. The supported marker annotation | ||
* formats are: | ||
* - `<code-identifier>` - the deviation applies to results on the current line. | ||
* - `codeql::<standard>_deviation(<code-identifier>)` - same as above. | ||
* - `codeql::<standard>_deviation_next_line(<code-identifier>)` - this deviation applies to results on the next line. | ||
* - `codeql::<standard>_deviation_begin(<code-identifier>)` - marks the beginning of a range of lines where the deviation applies. | ||
* - `codeql::<standard>_deviation_end(<code-identifier>)` - marks the end of a range of lines where the deviation applies. | ||
* | ||
* The valid `code-identifier`s are specified in deviation records, which also specify the query whose results are | ||
* suppressed by the deviation. | ||
* | ||
* For begin/end, we maintain a stack of begin markers. When we encounter an end marker, we pop the stack to determine | ||
* the range of that begin/end marker. If the stack is empty, the end marker is considered unmatched and invalid. If | ||
* the stack is non-empty at the end of the file, all the begin markers are considered unmatched and invalid. | ||
* | ||
* Begin/end markers are not valid across include boundaries, as the stack is not maintained across files. | ||
*/ | ||
|
||
import cpp | ||
import Deviations | ||
|
||
string supportedStandard() { result = ["misra", "autosar", "cert"] } | ||
|
||
/** | ||
* Holds if the given comment contains the code identifier. | ||
*/ | ||
bindingset[codeIdentifier] | ||
private predicate commentMatches(Comment comment, string codeIdentifier) { | ||
exists(string text | | ||
comment instanceof CppStyleComment and | ||
// strip the beginning slashes | ||
text = comment.getContents().suffix(2).trim() | ||
or | ||
comment instanceof CStyleComment and | ||
// strip both the beginning /* and the end */ the comment | ||
exists(string text0 | | ||
text0 = comment.getContents().suffix(2) and | ||
text = text0.prefix(text0.length() - 2).trim() | ||
) and | ||
// The /* */ comment must be a single-line comment | ||
not text.matches("%\n%") | ||
| | ||
// Code identifier appears at the start of the comment (modulo whitespace) | ||
text.prefix(codeIdentifier.length()) = codeIdentifier | ||
or | ||
// Code identifier appears at the end of the comment (modulo whitespace) | ||
text.suffix(text.length() - codeIdentifier.length()) = codeIdentifier | ||
) | ||
} | ||
|
||
/** | ||
* A deviation marker in the code. | ||
*/ | ||
abstract class CommentDeviationMarker extends Comment { | ||
DeviationRecord record; | ||
|
||
/** | ||
* Gets the deviation record associated with this deviation marker. | ||
*/ | ||
DeviationRecord getRecord() { result = record } | ||
} | ||
|
||
/** | ||
* A deviation marker in a comment that is not a valid deviation marker. | ||
*/ | ||
class InvalidCommentDeviationMarker extends Comment { | ||
InvalidCommentDeviationMarker() { | ||
not this instanceof CommentDeviationMarker and | ||
commentMatches(this, "codeql::" + supportedStandard() + "_deviation") | ||
} | ||
} | ||
|
||
/** | ||
* A deviation marker for a deviation that applies to the current line. | ||
*/ | ||
class DeviationEndOfLineMarker extends CommentDeviationMarker { | ||
DeviationEndOfLineMarker() { | ||
commentMatches(this, | ||
"codeql::" + supportedStandard() + "_deviation(" + record.getCodeIdentifier() + ")") | ||
} | ||
} | ||
|
||
/** | ||
* A deviation marker for a deviation that applies to the next line. | ||
*/ | ||
class DeviationNextLineMarker extends CommentDeviationMarker { | ||
DeviationNextLineMarker() { | ||
commentMatches(this, | ||
"codeql::" + supportedStandard() + "_deviation_next_line(" + record.getCodeIdentifier() + ")") | ||
} | ||
} | ||
|
||
/** | ||
* A deviation marker for a deviation that applies to a range of lines | ||
*/ | ||
abstract class CommentDeviationRangeMarker extends CommentDeviationMarker { } | ||
|
||
/** | ||
* A deviation marker for a deviation that begins on this line. | ||
*/ | ||
class DeviationBegin extends CommentDeviationRangeMarker { | ||
DeviationBegin() { | ||
commentMatches(this, | ||
"codeql::" + supportedStandard() + "_deviation_begin(" + record.getCodeIdentifier() + ")") | ||
} | ||
} | ||
|
||
/** | ||
* A deviation marker for a deviation that ends on this line. | ||
*/ | ||
class DeviationEnd extends CommentDeviationRangeMarker { | ||
DeviationEnd() { | ||
commentMatches(this, | ||
"codeql::" + supportedStandard() + "_deviation_end(" + record.getCodeIdentifier() + ")") | ||
} | ||
} | ||
|
||
private predicate hasDeviationCommentFileOrdering( | ||
DeviationRecord record, CommentDeviationRangeMarker comment, File file, int index | ||
) { | ||
comment = | ||
rank[index](CommentDeviationRangeMarker c | | ||
c.getRecord() = record and | ||
file = c.getFile() | ||
| | ||
c order by c.getLocation().getStartLine(), c.getLocation().getStartColumn() | ||
) | ||
} | ||
|
||
private predicate mkBeginStack(DeviationRecord record, File file, BeginStack stack, int index) { | ||
// Stack is empty at the start | ||
index = 0 and | ||
stack = TEmptyBeginStack() and | ||
exists(CommentDeviationRangeMarker marker | | ||
marker.getRecord() = record and marker.getLocation().getFile() = file | ||
) | ||
or | ||
// Next token is begin, so push it to the stack | ||
exists(DeviationBegin begin, BeginStack prev | | ||
record = begin.getRecord() and | ||
hasDeviationCommentFileOrdering(record, begin, file, index) and | ||
mkBeginStack(record, file, prev, index - 1) and | ||
stack = TConsBeginStack(begin, prev) | ||
) | ||
or | ||
// Next token is end | ||
exists(DeviationEnd end, BeginStack prevStack | | ||
record = end.getRecord() and | ||
hasDeviationCommentFileOrdering(record, end, file, index) and | ||
mkBeginStack(record, file, prevStack, index - 1) | ||
| | ||
// There is, so pop the most recent begin off the stack | ||
prevStack = TConsBeginStack(_, stack) | ||
or | ||
// Error, no begin on the stack, ignore and continue | ||
prevStack = TEmptyBeginStack() and | ||
stack = TEmptyBeginStack() | ||
) | ||
} | ||
|
||
newtype TBeginStack = | ||
TConsBeginStack(DeviationBegin begin, TBeginStack prev) { | ||
lcartey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
exists(File file, int index | | ||
hasDeviationCommentFileOrdering(begin.getRecord(), begin, file, index) and | ||
mkBeginStack(begin.getRecord(), file, prev, index - 1) | ||
MichaelRFairhurst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
} or | ||
TEmptyBeginStack() | ||
|
||
private class BeginStack extends TBeginStack { | ||
string toString() { | ||
exists(DeviationBegin begin, BeginStack prev | this = TConsBeginStack(begin, prev) | | ||
result = "(" + begin + ", " + prev.toString() + ")" | ||
) | ||
or | ||
this = TEmptyBeginStack() and | ||
result = "()" | ||
} | ||
} | ||
|
||
predicate isDeviationRangePaired(DeviationRecord record, DeviationBegin begin, DeviationEnd end) { | ||
exists(File file, int index | | ||
record = end.getRecord() and | ||
hasDeviationCommentFileOrdering(record, end, file, index) and | ||
mkBeginStack(record, file, TConsBeginStack(begin, _), index - 1) | ||
) | ||
} | ||
|
||
/** | ||
* A standard attribute that either deviates a result. | ||
*/ | ||
class DeviationAttribute extends StdAttribute { | ||
DeviationRecord record; | ||
|
||
DeviationAttribute() { | ||
this.hasQualifiedName("codeql", supportedStandard() + "_deviation") and | ||
// Support multiple argument deviations | ||
"\"" + record.getCodeIdentifier() + "\"" = this.getAnArgument().getValueText() | ||
} | ||
|
||
DeviationRecord getDeviationRecord() { result = record } | ||
lcartey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
pragma[nomagic] | ||
Element getASuppressedElement() { | ||
result.(Type).getAnAttribute() = this | ||
or | ||
result.(Stmt).getAnAttribute() = this | ||
or | ||
result.(Variable).getAnAttribute() = this | ||
or | ||
result.(Function).getAnAttribute() = this | ||
or | ||
result.(Expr).getEnclosingStmt() = this.getASuppressedElement() | ||
or | ||
result.(Stmt).getParentStmt() = this.getASuppressedElement() | ||
or | ||
result.(Stmt).getEnclosingFunction() = this.getASuppressedElement() | ||
or | ||
result.(LocalVariable) = this.getASuppressedElement().(DeclStmt).getADeclaration() | ||
or | ||
result.(Function).getDeclaringType() = this.getASuppressedElement() | ||
or | ||
result.(Variable).getDeclaringType() = this.getASuppressedElement() | ||
or | ||
exists(LambdaExpression expr | | ||
expr = this.getASuppressedElement() and | ||
result = expr.getLambdaFunction() | ||
) | ||
or | ||
exists(Function f | | ||
f = this.getASuppressedElement() and | ||
// A suppression on the function should apply to the noexcept expression | ||
result = f.getADeclarationEntry().getNoExceptExpr() | ||
) | ||
} | ||
} | ||
|
||
/** | ||
* A deviation attribute that is not associated with any deviation record. | ||
*/ | ||
class InvalidDeviationAttribute extends StdAttribute { | ||
string unknownCodeIdentifier; | ||
|
||
InvalidDeviationAttribute() { | ||
this.hasQualifiedName("codeql", supportedStandard() + "_deviation") and | ||
"\"" + unknownCodeIdentifier + "\"" = this.getAnArgument().getValueText() and | ||
not exists(DeviationRecord record | record.getCodeIdentifier() = unknownCodeIdentifier) | ||
} | ||
|
||
string getAnUnknownCodeIdentifier() { result = unknownCodeIdentifier } | ||
} | ||
|
||
newtype TCodeIndentifierDeviation = | ||
TSingleLineDeviation(DeviationRecord record, Comment comment, string filepath, int suppressedLine) { | ||
( | ||
commentMatches(comment, record.getCodeIdentifier()) or | ||
comment.(DeviationEndOfLineMarker).getRecord() = record | ||
) and | ||
comment.getLocation().hasLocationInfo(filepath, suppressedLine, _, _, _) | ||
or | ||
comment.(DeviationNextLineMarker).getRecord() = record and | ||
comment.getLocation().hasLocationInfo(filepath, suppressedLine - 1, _, _, _) | ||
} or | ||
TMultiLineDeviation( | ||
DeviationRecord record, DeviationBegin beginComment, DeviationEnd endComment, string filepath, | ||
int suppressedStartLine, int suppressedEndLine | ||
) { | ||
isDeviationRangePaired(record, beginComment, endComment) and | ||
beginComment.getLocation().hasLocationInfo(filepath, suppressedStartLine, _, _, _) and | ||
endComment.getLocation().hasLocationInfo(filepath, suppressedEndLine, _, _, _) | ||
} or | ||
TCodeIdentifierDeviation(DeviationRecord record, DeviationAttribute attribute) { | ||
attribute.getDeviationRecord() = record | ||
} | ||
|
||
class CodeIdentifierDeviation extends TCodeIndentifierDeviation { | ||
/** The deviation record associated with the deviation comment. */ | ||
DeviationRecord getDeviationRecord() { | ||
this = TSingleLineDeviation(result, _, _, _) | ||
or | ||
this = TMultiLineDeviation(result, _, _, _, _, _) | ||
or | ||
this = TCodeIdentifierDeviation(result, _) | ||
} | ||
|
||
/** | ||
* Holds if the given element is matched by this code identifier deviation. | ||
*/ | ||
bindingset[e] | ||
pragma[inline_late] | ||
predicate isElementMatching(Element e) { | ||
exists(string filepath, int elementLocationStart | | ||
e.getLocation().hasLocationInfo(filepath, elementLocationStart, _, _, _) | ||
| | ||
exists(int suppressedLine | | ||
this = TSingleLineDeviation(_, _, filepath, suppressedLine) and | ||
suppressedLine = elementLocationStart | ||
) | ||
or | ||
exists(int suppressedStartLine, int suppressedEndLine | | ||
this = TMultiLineDeviation(_, _, _, filepath, suppressedStartLine, suppressedEndLine) and | ||
suppressedStartLine < elementLocationStart and | ||
suppressedEndLine > elementLocationStart | ||
) | ||
) | ||
or | ||
exists(DeviationAttribute attribute | | ||
this = TCodeIdentifierDeviation(_, attribute) and | ||
attribute.getASuppressedElement() = e | ||
) | ||
} | ||
|
||
string toString() { | ||
exists(string filepath | | ||
exists(int suppressedLine | | ||
this = TSingleLineDeviation(_, _, filepath, suppressedLine) and | ||
result = | ||
"Deviation record " + getDeviationRecord() + " applied to " + filepath + " Line " + | ||
suppressedLine | ||
) | ||
or | ||
exists(int suppressedStartLine, int suppressedEndLine | | ||
this = TMultiLineDeviation(_, _, _, filepath, suppressedStartLine, suppressedEndLine) and | ||
result = | ||
"Deviation record " + getDeviationRecord() + " applied to " + filepath + " Line" + | ||
suppressedStartLine + ":" + suppressedEndLine | ||
) | ||
) | ||
or | ||
exists(DeviationAttribute attribute | | ||
this = TCodeIdentifierDeviation(_, attribute) and | ||
result = "Deviation record " + getDeviationRecord() + " applied to " + attribute | ||
) | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.