Skip to content
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

Add support for deviations on next line and multiple lines #807

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions cpp/common/src/codingstandards/cpp/Exclusions.qll
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,14 @@ predicate isExcluded(Element e, Query query, string reason) {
) and
reason = "Query has an associated deviation record for the element's file."
or
// The element is on the same line as a suppression comment
exists(Comment c |
c = dr.getACodeIdentifierComment() and
query = dr.getQuery()
|
exists(string filepath, int endLine |
// Comment occurs on the same line as the end line of the element
e.getLocation().hasLocationInfo(filepath, _, _, endLine, _) and
c.getLocation().hasLocationInfo(filepath, endLine, _, _, _)
)
) and
reason =
"Query has an associated deviation record with a code identifier that is applied to the element."
// The element is annotated by a code identifier that deviates this rule
exists(CodeIdentifierDeviation deviationInCode |
dr.getQuery() = query and
deviationInCode = dr.getACodeIdentifierDeviation() and
deviationInCode.isElementMatching(e) and
reason =
"Query has an associated deviation record with a code identifier that is applied to the element."
)
)
or
// The effective category of the query is 'Disapplied'.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
/**
* A module for identifying comment markers in code that trigger deviations.
*
* Each comment 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.
* - `DEVIATION(<code-identifier>)` - same as above.
* - `DEVIATION_NEXT_LINE(<code-identifier>)` - this deviation applies to results on the next line.
* - `DEVIATION_BEGIN(<code-identifier>)` - marks the beginning of a range of lines where the deviation applies.
* - `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

/**
* 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 DeviationMarker extends Comment {
DeviationRecord record;

/**
* Gets the deviation record associated with this deviation marker.
*/
DeviationRecord getRecord() { result = record }
}

/**
* A deviation marker for a deviation that applies to the current line.
*/
class DeviationEndOfLineMarker extends DeviationMarker {
DeviationEndOfLineMarker() {
commentMatches(this, "DEVIATION(" + record.getCodeIdentifier() + ")")
}
}

/**
* A deviation marker for a deviation that applies to the next line.
*/
class DeviationNextLineMarker extends DeviationMarker {
DeviationNextLineMarker() {
commentMatches(this, "DEVIATION_NEXT_LINE(" + record.getCodeIdentifier() + ")")
}
}

/**
* A deviation marker for a deviation that applies to a range of lines
*/
abstract class DeviationRangeMarker extends DeviationMarker { }

/**
* A deviation marker for a deviation that begins on this line.
*/
class DeviationBegin extends DeviationRangeMarker {
DeviationBegin() { commentMatches(this, "DEVIATION_BEGIN(" + record.getCodeIdentifier() + ")") }
}

/**
* A deviation marker for a deviation that ends on this line.
*/
class DeviationEnd extends DeviationRangeMarker {
DeviationEnd() { commentMatches(this, "DEVIATION_END(" + record.getCodeIdentifier() + ")") }
}

private predicate hasDeviationCommentFileOrdering(
DeviationRecord record, DeviationRangeMarker comment, File file, int index
) {
comment =
rank[index](DeviationRangeMarker 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(DeviationRangeMarker 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) {
exists(File file, int index |
hasDeviationCommentFileOrdering(begin.getRecord(), begin, file, index) and
mkBeginStack(begin.getRecord(), file, prev, index - 1)
)
} 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 = "()"
}
}

private 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)
)
}

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, _, _, _)
}

class CodeIdentifierDeviation extends TCodeIndentifierDeviation {
/** The deviation record associated with the deviation comment. */
DeviationRecord getDeviationRecord() {
this = TSingleLineDeviation(result, _, _, _)
or
this = TMultiLineDeviation(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
)
)
}

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
)
)
}
}
29 changes: 3 additions & 26 deletions cpp/common/src/codingstandards/cpp/deviations/Deviations.qll
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import cpp
import semmle.code.cpp.XML
import codingstandards.cpp.exclusions.RuleMetadata
import codingstandards.cpp.Config
import CodeIdentifierDeviation

predicate applyDeviationsAtQueryLevel() {
not exists(CodingStandardsReportDeviatedAlerts reportDeviatedResults |
Expand Down Expand Up @@ -219,32 +220,8 @@ class DeviationRecord extends XmlElement {
else result = getADeviationPermit().getCodeIdentifier()
}

/** Gets a comment which starts or ends with the code identifier comment. */
Comment getACodeIdentifierComment() {
exists(string text |
(
result instanceof CppStyleComment and
// strip the beginning slashes
text = result.getContents().suffix(2).trim()
or
result instanceof CStyleComment and
// strip both the beginning /* and the end */ the comment
exists(string text0 |
text0 = result.getContents().suffix(2) and
text = text0.prefix(text0.length() - 2).trim()
) and
// The /* */ comment must be a single-line comment
not text.matches("%\n%")
) and
(
// Code identifier appears at the start of the comment (modulo whitespace)
text.prefix(getCodeIdentifier().length()) = getCodeIdentifier()
or
// Code identifier appears at the end of the comment (modulo whitespace)
text.suffix(text.length() - getCodeIdentifier().length()) = getCodeIdentifier()
)
)
}
/** Gets a code identifier deviation in code which starts or ends with the code identifier comment. */
CodeIdentifierDeviation getACodeIdentifierDeviation() { this = result.getDeviationRecord() }

/** Gets the `rule-id` specified for this record, if any. */
private string getRawRuleId() { result = getAChild("rule-id").getTextValue() }
Expand Down

This file was deleted.

Loading
Loading