mirror of https://github.com/mongodb/mongo
SERVER-103322 noexcept checker: don't alert for moves (#44718)
GitOrigin-RevId: 9d6a563d17af8e019cd548c6117ea54bde45bd8e
This commit is contained in:
Ryan Berryhill
MongoDB Bot
parent
99d4b6d871
commit
079c149385
|
|
@ -3,13 +3,16 @@
|
|||
Check for changes in noexcept usage between the given revisions and output a file that
|
||||
can be used by evergreen to post a github commit check annotating a PR with the results.
|
||||
|
||||
The algorithm here basically handles two cases: (1) straightforward cases where
|
||||
you're "just" adding or removing noexcept from an existing function; and (2)
|
||||
some cases where you're adding a new noexcept function or functions. Case (1)
|
||||
works like this: if we have a diff block (that is, a block of addition or
|
||||
deletion) that's a single line and mentions noexcept on only one side, count
|
||||
it. Case (2) works like this: if noexcept is mentioned on the b-side of the
|
||||
diff and not the a-side, assume new noexcept code has been added, and count it.
|
||||
The algorithm here handles a few cases: (1) straightforward cases where you're "just"
|
||||
adding or removing noexcept from an existing function; (2) some cases where you're adding
|
||||
a new noexcept function or functions; (3) cases where you're adding a non-noexcept move
|
||||
constructor or assignment operation. Case (1) works like this: if we have a diff block
|
||||
(that is, a block of addition or deletion) that's only mentions noexcept on only one side,
|
||||
those mentions are not detected as move operations, and the only diff between the two
|
||||
sides is related to the noexcept(s), count it. Case (2) works like this: if noexcept is
|
||||
mentioned more times on the b-side of the diff than the a-side (excluding move
|
||||
operations), assume new noexcept code has been added, and count it. Case (3) simply
|
||||
detects added code that looks like a move operation and alerts if it isn't noexcept.
|
||||
"""
|
||||
|
||||
import difflib
|
||||
|
|
@ -45,8 +48,8 @@ SUMMARY_TEXT = (
|
|||
|
||||
# These fields go in the annotations in the "Files Changed" tab. These fields are rendered
|
||||
# verbatim (i.e., markdown doesn't work).
|
||||
ANNOTATION_TITLE_TEXT = TITLE_TEXT
|
||||
ANNOTATION_MESSAGE_TEXT = (
|
||||
ANNOTATION_TITLE_TEXT_NOEXCEPT_CHANGE = TITLE_TEXT
|
||||
ANNOTATION_MESSAGE_TEXT_NOEXCEPT_CHANGE = (
|
||||
"A change in noexcept usage has been detected.\n"
|
||||
"As noexcept is a frequently-misunderstood feature,\n"
|
||||
"please refer to the documentation on noexcept usage\n"
|
||||
|
|
@ -61,6 +64,17 @@ ANNOTATION_MESSAGE_TEXT = (
|
|||
"Documentation can be found here: " + DOCUMENTATION_URL
|
||||
)
|
||||
|
||||
ANNOTATION_TITLE_TEXT_NON_NOEXCEPT_MOVE = "Non-noexcept move operation added"
|
||||
ANNOTATION_MESSAGE_TEXT_NON_NOEXCEPT_MOVE = (
|
||||
"A non-noexcept move operation has been added in this change.\n"
|
||||
"Move operations should generally be marked noexcept to\n"
|
||||
"avoid unnecessary copies during standard library operations.\n"
|
||||
"If the move operation truly can throw and is intentionally not\n"
|
||||
"noexcept, please disregard this message. "
|
||||
"\n\n"
|
||||
"Documentation can be found here: " + DOCUMENTATION_URL
|
||||
)
|
||||
|
||||
|
||||
def configure_logging() -> None:
|
||||
logging.basicConfig(
|
||||
|
|
@ -82,43 +96,45 @@ def get_git_diff(repo: git.Repo, base: git.Commit, rev) -> git.DiffIndex:
|
|||
return base.diff(rev)
|
||||
|
||||
|
||||
class ChangeKind(Enum):
|
||||
ADDITION = auto()
|
||||
REMOVAL = auto()
|
||||
def get_matching_blocks(lhs: str, rhs: str) -> list[tuple[int, int, int, int]]:
|
||||
"""
|
||||
Find the (line-level) matching blocks between the two given strings, representing the left-hand
|
||||
side (lhs) and right-hand side (rhs) of a file with a diff. That is, they represent the file's
|
||||
contents before (lhs) and after (rhs) the change. Return value is a tuple of (lhs_start,
|
||||
lhs_end, rhs_start, rhs_end) for each matching block. Unlike difflib's
|
||||
SequenceMatcher.get_matching_blocks, we do not return the dummy size-0 block at the end.
|
||||
"""
|
||||
lhs_lines = lhs.splitlines()
|
||||
rhs_lines = rhs.splitlines()
|
||||
matcher = difflib.SequenceMatcher(None, lhs_lines, rhs_lines, autojunk=False)
|
||||
blocks = matcher.get_matching_blocks()
|
||||
line_diffs = []
|
||||
for block in blocks:
|
||||
if block.size == 0:
|
||||
# The last block is a dummy block a and b set to the sequence lengths and size 0.
|
||||
# We don't return such a block since we don't need it.
|
||||
break
|
||||
lhs_start = block.a
|
||||
rhs_start = block.b
|
||||
line_diffs.append((lhs_start, lhs_start + block.size, rhs_start, rhs_start + block.size))
|
||||
return line_diffs
|
||||
|
||||
|
||||
@dataclass
|
||||
class UnboundNoexceptChange:
|
||||
kind: ChangeKind
|
||||
is_certain: bool
|
||||
index: int
|
||||
|
||||
def bind_to_line(self, line: int):
|
||||
return LineBoundNoexceptChange(**asdict(self), line=line)
|
||||
|
||||
|
||||
@dataclass
|
||||
class LineBoundNoexceptChange(UnboundNoexceptChange):
|
||||
line: int
|
||||
|
||||
def bind_to_file(self, file: str):
|
||||
return FileBoundNoexceptChange(**asdict(self), file=file)
|
||||
|
||||
|
||||
@dataclass
|
||||
class FileBoundNoexceptChange(LineBoundNoexceptChange):
|
||||
file: str
|
||||
|
||||
def make_annotation(self) -> dict[str, Any]:
|
||||
return {
|
||||
"path": self.file,
|
||||
"annotation_level": "notice",
|
||||
"title": ANNOTATION_TITLE_TEXT,
|
||||
"message": ANNOTATION_MESSAGE_TEXT,
|
||||
"raw_details": "",
|
||||
"start_line": self.line,
|
||||
"end_line": self.line,
|
||||
}
|
||||
def get_nonmatching_blocks(lhs: str, rhs: str) -> list[tuple[int, int, int, int]]:
|
||||
matches = get_matching_blocks(lhs, rhs)
|
||||
lhs_index = 0
|
||||
rhs_index = 0
|
||||
blocks = []
|
||||
lhs_line_count = len(lhs.splitlines())
|
||||
rhs_line_count = len(rhs.splitlines())
|
||||
for lhs_start, lhs_end, rhs_start, rhs_end in matches:
|
||||
if lhs_index < lhs_start or rhs_index < rhs_start:
|
||||
blocks.append((lhs_index, lhs_start, rhs_index, rhs_start))
|
||||
lhs_index = lhs_end
|
||||
rhs_index = rhs_end
|
||||
if lhs_index < lhs_line_count or rhs_index < rhs_line_count:
|
||||
blocks.append((lhs_index, lhs_line_count, rhs_index, rhs_line_count))
|
||||
return blocks
|
||||
|
||||
|
||||
def get_line_for_char(text: str, i: int) -> int:
|
||||
|
|
@ -126,148 +142,280 @@ def get_line_for_char(text: str, i: int) -> int:
|
|||
return text[:i].count("\n") + 1
|
||||
|
||||
|
||||
def analyze_uncertain_insdel(insdel: str, kind: ChangeKind) -> list[UnboundNoexceptChange]:
|
||||
return [
|
||||
UnboundNoexceptChange(kind=kind, is_certain=False, index=match.start())
|
||||
for match in re.finditer("noexcept", insdel)
|
||||
def strip_noexcept(s: str) -> str:
|
||||
return re.sub(r"\snoexcept(\s*\(.*\))?(?=\s|;|{|$)", "", s)
|
||||
|
||||
|
||||
class SnippetKind(Enum):
|
||||
# Noexcept is the most general kind of snippet, it simply refers to an instance of noexcept
|
||||
# that wasn't classified as more specific type of snippet.
|
||||
Noexcept = auto()
|
||||
# A NoexceptAddition is a snippet representing a definite addition of noexcept that wasn't
|
||||
# classified as a move.
|
||||
NoexceptAddition = auto()
|
||||
# A NoexceptRemoval is a snippet representing a definite removal of noexcept that wasn't
|
||||
# classified as a move.
|
||||
NoexceptRemoval = auto()
|
||||
# A NoexceptMove is a move operation that is marked noexcept.
|
||||
NoexceptMove = auto()
|
||||
# A NonNoexceptMove is a move operation that is not marked noexcept.
|
||||
NonNoexceptMove = auto()
|
||||
|
||||
|
||||
@dataclass
|
||||
class UnboundSnippet:
|
||||
kind: SnippetKind
|
||||
start: int
|
||||
end: int
|
||||
identifier: str | None
|
||||
|
||||
def is_move(self) -> bool:
|
||||
return self.kind in (SnippetKind.NoexceptMove, SnippetKind.NonNoexceptMove)
|
||||
|
||||
# We take a separate alert line since snippets on the LHS that are alertable need to be attached
|
||||
# to a line on the RHS. If it's not provided, we just use the start line of the snippet,
|
||||
# under the assumption that it's from the RHS.
|
||||
def _bind_to_text(self, s: str, alert_line: int | None = None):
|
||||
start = get_line_for_char(s, self.start)
|
||||
end = get_line_for_char(s, self.end)
|
||||
if alert_line is None:
|
||||
alert_line = start
|
||||
return LineBoundSnippet(
|
||||
**asdict(self), line_start=start, line_end=end, alert_line=alert_line
|
||||
)
|
||||
|
||||
def bind_to_rhs(self, s: str):
|
||||
return self._bind_to_text(s)
|
||||
|
||||
def bind_to_lhs(self, s: str, alert_line: int):
|
||||
return self._bind_to_text(s, alert_line=alert_line)
|
||||
|
||||
|
||||
@dataclass
|
||||
class LineBoundSnippet(UnboundSnippet):
|
||||
line_start: int
|
||||
line_end: int
|
||||
alert_line: int
|
||||
|
||||
def bind_to_file(self, file: str):
|
||||
return FileBoundSnippet(**asdict(self), file=file)
|
||||
|
||||
|
||||
@dataclass
|
||||
class FileBoundSnippet(LineBoundSnippet):
|
||||
file: str
|
||||
|
||||
|
||||
class AlertKind(Enum):
|
||||
NonNoexceptMove = auto()
|
||||
Noexcept = auto()
|
||||
|
||||
|
||||
@dataclass
|
||||
class Alert:
|
||||
file: str
|
||||
line: int
|
||||
kind: AlertKind
|
||||
identifier: str | None
|
||||
|
||||
def get_title(self) -> str:
|
||||
if self.kind == AlertKind.Noexcept:
|
||||
return ANNOTATION_TITLE_TEXT_NOEXCEPT_CHANGE
|
||||
else:
|
||||
assert self.kind == AlertKind.NonNoexceptMove
|
||||
return ANNOTATION_TITLE_TEXT_NON_NOEXCEPT_MOVE
|
||||
|
||||
def get_message(self) -> str:
|
||||
if self.kind == AlertKind.Noexcept:
|
||||
return ANNOTATION_MESSAGE_TEXT_NOEXCEPT_CHANGE
|
||||
else:
|
||||
assert self.kind == AlertKind.NonNoexceptMove
|
||||
return ANNOTATION_MESSAGE_TEXT_NON_NOEXCEPT_MOVE
|
||||
|
||||
def make_annotation(self) -> dict[str, Any]:
|
||||
return {
|
||||
"path": self.file,
|
||||
"annotation_level": "notice",
|
||||
"title": self.get_title(),
|
||||
"message": self.get_message(),
|
||||
"raw_details": "",
|
||||
"start_line": self.line,
|
||||
"end_line": self.line,
|
||||
}
|
||||
|
||||
|
||||
def get_noexcepts(s: str) -> list[UnboundSnippet]:
|
||||
noexcept_patterns = [
|
||||
re.compile(r"\bnoexcept\b"),
|
||||
]
|
||||
noexcepts = []
|
||||
for pattern in noexcept_patterns:
|
||||
for match in pattern.finditer(s):
|
||||
snippet = UnboundSnippet(
|
||||
start=match.start(),
|
||||
end=match.end(),
|
||||
identifier=None,
|
||||
kind=SnippetKind.Noexcept,
|
||||
)
|
||||
noexcepts.append(snippet)
|
||||
return noexcepts
|
||||
|
||||
|
||||
def _get_all_move_operations(s: str) -> list[tuple[int, int, str]]:
|
||||
"""
|
||||
Find all move constructors and assignment operators in the given string. Returns a list of
|
||||
(start_char, end_char, identifier) triples, where identifier is what we detected as the
|
||||
name of the move operation (e.g., TypeName::TypeName for a move constructor or
|
||||
TypeName::operator= for a move assignment).
|
||||
"""
|
||||
move_constructor_patterns = [
|
||||
# TypeName(TypeName&&...) - TypeName can be anything capitalized and we don't check the two
|
||||
# instances of Typename are the same. We also don't check that TypeName is the name of the
|
||||
# type, so this could be a normal function with a bad name. We do try to check that there
|
||||
# are no additional parameters beyond the move parameter.
|
||||
re.compile(r"\b[A-Z]\w*\s*\(([A-Z]\w*)\s*&&[^,)]*\)", re.MULTILINE),
|
||||
]
|
||||
move_assignment_patterns = [
|
||||
# operator=(Typename&&...) - TypeName can be anything capitalized and we do try to check
|
||||
# that there are no additional parameters beyond the move parameter.
|
||||
re.compile(r"\boperator=\s*\(([A-Z]\w*)\s*&&[^,)]*\)", re.MULTILINE),
|
||||
]
|
||||
|
||||
move_operations = []
|
||||
|
||||
def analyze_insdel(insdel: str, kind: ChangeKind) -> list[UnboundNoexceptChange]:
|
||||
# If the inserted or deleted string is a single line, we'll assume the presence of noexcept
|
||||
# indicates a true insertion or deletion of noexcept.
|
||||
if insdel.count("noexcept") == 1 and insdel.count("\n") == 0:
|
||||
return [UnboundNoexceptChange(kind=kind, is_certain=True, index=insdel.find("noexcept"))]
|
||||
return analyze_uncertain_insdel(insdel, kind)
|
||||
for pattern in move_constructor_patterns:
|
||||
for match in pattern.finditer(s):
|
||||
move_operations.append(
|
||||
(match.start(), match.end(), match.group(1) + "::" + match.group(1))
|
||||
)
|
||||
|
||||
for pattern in move_assignment_patterns:
|
||||
for match in pattern.finditer(s):
|
||||
move_operations.append((match.start(), match.end(), match.group(1) + "::operator="))
|
||||
|
||||
return move_operations
|
||||
|
||||
|
||||
def analyze_deletion(deleted: str) -> list[UnboundNoexceptChange]:
|
||||
return analyze_insdel(deleted, ChangeKind.REMOVAL)
|
||||
def get_move_operations(s: str) -> list[UnboundSnippet]:
|
||||
"""
|
||||
Find and return all move operations (constructors or assignment operators) in the given string.
|
||||
"""
|
||||
matches = _get_all_move_operations(s)
|
||||
snippets = []
|
||||
for start, end, identifier in matches:
|
||||
# end is the closing bracket of the move operation's parameter list.
|
||||
# We look for the noexcept token between there and the next ;, {, or end of string,
|
||||
# and we'll also extend the snippet to that point if noexcept is found.
|
||||
move_op_end = min([s.find(c, end) if c in s else len(s) for c in (";", "{")])
|
||||
trailing_text = s[end:move_op_end]
|
||||
is_noexcept = "noexcept" in trailing_text
|
||||
snippet_kind = SnippetKind.NoexceptMove if is_noexcept else SnippetKind.NonNoexceptMove
|
||||
snippet_end = move_op_end if is_noexcept else end
|
||||
snippet = UnboundSnippet(
|
||||
start=start,
|
||||
end=snippet_end,
|
||||
identifier=identifier,
|
||||
kind=snippet_kind,
|
||||
)
|
||||
snippets.append(snippet)
|
||||
|
||||
return snippets
|
||||
|
||||
|
||||
def analyze_insertion(inserted: str) -> list[UnboundNoexceptChange]:
|
||||
return analyze_insdel(inserted, ChangeKind.ADDITION)
|
||||
def filter_snippets(
|
||||
snippets: list[LineBoundSnippet], start: int, end: int
|
||||
) -> list[LineBoundSnippet]:
|
||||
def is_in_range(snippet: LineBoundSnippet, start: int, end: int) -> bool:
|
||||
return start <= snippet.line_start <= end or start <= snippet.line_end <= end
|
||||
|
||||
return [s for s in snippets if is_in_range(s, start, end)]
|
||||
|
||||
|
||||
def analyze_edit(deleted: str, inserted: str) -> list[UnboundNoexceptChange]:
|
||||
if "noexcept" not in inserted:
|
||||
return analyze_deletion(deleted)
|
||||
if "noexcept" not in deleted:
|
||||
return analyze_insertion(inserted)
|
||||
def drop_covered_noexcept_snippets(snippets: list[LineBoundSnippet]) -> list[LineBoundSnippet]:
|
||||
"""
|
||||
Remove any snippets of less-specific types (Noexcept, NoexceptAddition, NoexceptRemoval) that
|
||||
are fully covered by snippets of more-specific types (NoexceptMove).
|
||||
"""
|
||||
|
||||
# Both sides mention noexcept - we have far less certainty.
|
||||
deletions = analyze_uncertain_insdel(deleted, ChangeKind.REMOVAL)
|
||||
insertions = analyze_uncertain_insdel(inserted, ChangeKind.ADDITION)
|
||||
|
||||
return insertions + deletions
|
||||
noexcept_moves = [s for s in snippets if s.kind == SnippetKind.NoexceptMove]
|
||||
filtered = []
|
||||
for snippet in snippets:
|
||||
if snippet.is_move():
|
||||
filtered.append(snippet)
|
||||
continue
|
||||
is_covered = any(
|
||||
move_op.start <= snippet.start <= move_op.end for move_op in noexcept_moves
|
||||
)
|
||||
if not is_covered:
|
||||
filtered.append(snippet)
|
||||
return filtered
|
||||
|
||||
|
||||
# This is a wrapper around difflib.SeqeuenceMatcher's get_matching_blocks.
|
||||
# SequenceMatcher synchronizes the sequences quite aggressively when diffing a character sequence,
|
||||
# often inserting a very small match in the middle of what we'd like to consider a single edit.
|
||||
# For example, considering the strings "int f() noexcept const" and "int f() const",
|
||||
# it might align them like this:
|
||||
# int f() noexcept const
|
||||
# int f() c onst
|
||||
# producing two insertions of "noex" and "ept", making it hard for us to analyze individual edits.
|
||||
# This wrapper performs the match at the level of words and translates back to a character-based
|
||||
# match sequence. The matching ignores whitespace, so matching regions may be of different sizes
|
||||
# on each side and may indeed differ in whitespace.
|
||||
def get_matching_blocks(lhs: str, rhs: str) -> list[tuple[int, int, int, int]]:
|
||||
def skip_whitespace(s: str, i: int) -> int:
|
||||
while i < len(s) and s[i].isspace():
|
||||
i += 1
|
||||
return i
|
||||
def analyze_text_diff(lhs: str, rhs: str) -> tuple[list[LineBoundSnippet], list[LineBoundSnippet]]:
|
||||
"""
|
||||
Analyze the text diff between lhs and rhs, returning snippets of noexcept-relevant changes.
|
||||
Returns two lists: snippets from lhs and snippets from rhs.
|
||||
"""
|
||||
|
||||
def skip_word(s: str, word: str, i: int) -> int:
|
||||
for char in word:
|
||||
assert i < len(s)
|
||||
assert s[i] == char
|
||||
i += 1
|
||||
return i
|
||||
def normalize(s: str) -> str:
|
||||
return re.sub(r"\s+", "", s)
|
||||
|
||||
def advance(s: str, word: str, i: int) -> int:
|
||||
i = skip_whitespace(s, i)
|
||||
i = skip_word(s, word, i)
|
||||
i = skip_whitespace(s, i)
|
||||
return i
|
||||
line_diff = get_nonmatching_blocks(lhs, rhs)
|
||||
|
||||
lhs_words = lhs.split()
|
||||
rhs_words = rhs.split()
|
||||
s = difflib.SequenceMatcher(None, lhs_words, rhs_words, autojunk=False)
|
||||
matching_blocks = s.get_matching_blocks()
|
||||
lhs_word_index = 0
|
||||
lhs_char_index = 0
|
||||
rhs_word_index = 0
|
||||
rhs_char_index = 0
|
||||
new_blocks = []
|
||||
for block in matching_blocks:
|
||||
if block.size == 0:
|
||||
# The last block is a dummy block a and b set to the sequence lengths and size 0.
|
||||
# We don't return such a block since we don't need it.
|
||||
break
|
||||
# Get all snippets, considering the whole file. We'll filter down to those that overlap with
|
||||
# each diff block later. This ensures that when finding snippets, we have enough context.
|
||||
# For example, if there's code like:
|
||||
# TypeName& operator=(TypeName&& other)
|
||||
# noexcept {
|
||||
# and the diff only covers the "noexcept {" line, we still want to look at the line before
|
||||
# to realize it's a move assignment operator.
|
||||
all_pre_snippets = drop_covered_noexcept_snippets(get_noexcepts(lhs) + get_move_operations(lhs))
|
||||
all_post_snippets = drop_covered_noexcept_snippets(
|
||||
get_noexcepts(rhs) + get_move_operations(rhs)
|
||||
)
|
||||
|
||||
while lhs_word_index < block.a:
|
||||
lhs_char_index = advance(lhs, lhs_words[lhs_word_index], lhs_char_index)
|
||||
lhs_word_index += 1
|
||||
while rhs_word_index < block.b:
|
||||
rhs_char_index = advance(rhs, rhs_words[rhs_word_index], rhs_char_index)
|
||||
rhs_word_index += 1
|
||||
pre, post = [], []
|
||||
for lstart, lend, rstart, rend in line_diff:
|
||||
# Ensure snippets on the LHS generate alerts on the corresponding RHS line, as alerts
|
||||
# can only appear on the RHS. Snippets already on the RHS just use their own line.
|
||||
pre_snippets = [s.bind_to_lhs(lhs, rstart) for s in all_pre_snippets]
|
||||
post_snippets = [s.bind_to_rhs(rhs) for s in all_post_snippets]
|
||||
|
||||
block_lhs_start = lhs_char_index
|
||||
while lhs_word_index < block.a + block.size:
|
||||
lhs_char_index = advance(lhs, lhs_words[lhs_word_index], lhs_char_index)
|
||||
lhs_word_index += 1
|
||||
block_rhs_start = rhs_char_index
|
||||
while rhs_word_index < block.b + block.size:
|
||||
rhs_char_index = advance(rhs, rhs_words[rhs_word_index], rhs_char_index)
|
||||
rhs_word_index += 1
|
||||
# Filter snippets down to those that overlap with this diff block.
|
||||
pre_snippets = filter_snippets(pre_snippets, lstart, lend)
|
||||
post_snippets = filter_snippets(post_snippets, rstart, rend)
|
||||
|
||||
new_blocks.append((block_lhs_start, lhs_char_index, block_rhs_start, rhs_char_index))
|
||||
rhs_text = " ".join(rhs.splitlines()[rstart:rend])
|
||||
lhs_text = " ".join(lhs.splitlines()[lstart:lend])
|
||||
|
||||
return new_blocks
|
||||
# Special case: if there are snippets on only one side, they're all of type Noexcept,
|
||||
# and the only textual difference between the two sides is the presence or absence of
|
||||
# noexcept, we can promote those snippets to NoexceptAddition or NoexceptRemoval.
|
||||
# This is the case where we can definitively tell that noexcept was added to or removed
|
||||
# from existing code, and that code isn't a move operation (probably, up to the accuracy
|
||||
# of our move detection).
|
||||
if (
|
||||
len(pre_snippets) == 0
|
||||
and len(post_snippets) > 0
|
||||
and all(s.kind == SnippetKind.Noexcept for s in post_snippets)
|
||||
and normalize(strip_noexcept(rhs_text)) == normalize(lhs_text)
|
||||
):
|
||||
for s in post_snippets:
|
||||
s.kind = SnippetKind.NoexceptAddition
|
||||
|
||||
if (
|
||||
len(post_snippets) == 0
|
||||
and len(pre_snippets) > 0
|
||||
and all(s.kind == SnippetKind.Noexcept for s in pre_snippets)
|
||||
and normalize(strip_noexcept(lhs_text)) == normalize(rhs_text)
|
||||
):
|
||||
for s in pre_snippets:
|
||||
s.kind = SnippetKind.NoexceptRemoval
|
||||
|
||||
def get_nonmatching_blocks(lhs: str, rhs: str) -> list[tuple[int, int, int, int]]:
|
||||
matches = get_matching_blocks(lhs, rhs)
|
||||
lhs_i = 0
|
||||
rhs_i = 0
|
||||
blocks = []
|
||||
for lhs_start, lhs_end, rhs_start, rhs_end in matches:
|
||||
if lhs_i < lhs_start or rhs_i < rhs_start:
|
||||
blocks.append((lhs_i, lhs_start, rhs_i, rhs_start))
|
||||
lhs_i = lhs_end
|
||||
rhs_i = rhs_end
|
||||
if lhs_i < len(lhs) or rhs_i < len(rhs):
|
||||
blocks.append((lhs_i, len(lhs), rhs_i, len(rhs)))
|
||||
return blocks
|
||||
pre += pre_snippets
|
||||
post += post_snippets
|
||||
|
||||
|
||||
def analyze_text_diff(lhs: str, rhs: str) -> list[LineBoundNoexceptChange]:
|
||||
blocks = get_nonmatching_blocks(lhs, rhs)
|
||||
changes = []
|
||||
for lhs_start, lhs_end, rhs_start, rhs_end in blocks:
|
||||
deleted = lhs[lhs_start:lhs_end]
|
||||
inserted = rhs[rhs_start:rhs_end]
|
||||
block_changes = analyze_edit(deleted, inserted)
|
||||
|
||||
# Line number always refers to the rhs of the diff (post-changes). Annotations can only
|
||||
# be put on that side.
|
||||
rhs_start_line = get_line_for_char(rhs, rhs_start)
|
||||
|
||||
# Indices are relative to the start of inserted/deleted, convert them to be relative
|
||||
# to lhs/rhs.
|
||||
for change in block_changes:
|
||||
if change.kind == ChangeKind.ADDITION:
|
||||
# For additions, we bind to the line where noexcept appears.
|
||||
change.index += rhs_start
|
||||
changes += [change.bind_to_line(get_line_for_char(rhs, change.index))]
|
||||
elif change.kind == ChangeKind.REMOVAL:
|
||||
# For removals, we bind to the start of the diff block due to the constraint that
|
||||
# annotations go on the right-hand side.
|
||||
change.index += lhs_start
|
||||
changes += [change.bind_to_line(rhs_start_line)]
|
||||
return changes
|
||||
return pre, post
|
||||
|
||||
|
||||
def is_allowed_path(path: str) -> bool:
|
||||
|
|
@ -278,7 +426,9 @@ def is_allowed_path(path: str) -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def analyze_single_git_diff(diff: git.Diff) -> list[LineBoundNoexceptChange]:
|
||||
def analyze_single_git_diff(
|
||||
diff: git.Diff,
|
||||
) -> tuple[list[LineBoundSnippet], list[LineBoundSnippet]]:
|
||||
# Blobs can be None. For example, if the diff is a file deletion, the b_blob will be None.
|
||||
# In that case, we want the None to be treated as empty string for our text diff.
|
||||
def decode(blob: git.Blob | None) -> str:
|
||||
|
|
@ -287,24 +437,108 @@ def analyze_single_git_diff(diff: git.Diff) -> list[LineBoundNoexceptChange]:
|
|||
return blob.data_stream.read().decode("utf-8")
|
||||
|
||||
if not is_allowed_path(diff.a_path):
|
||||
return []
|
||||
return [], []
|
||||
if not is_allowed_path(diff.b_path):
|
||||
return []
|
||||
return [], []
|
||||
|
||||
return analyze_text_diff(decode(diff.a_blob), decode(diff.b_blob))
|
||||
|
||||
|
||||
def analyze_git_diff(diffs: list[git.Diff]) -> list[FileBoundNoexceptChange]:
|
||||
changes = []
|
||||
def analyze_git_diff(
|
||||
diffs: list[git.Diff],
|
||||
) -> tuple[list[FileBoundSnippet], list[FileBoundSnippet]]:
|
||||
pre, post = [], []
|
||||
for diff in diffs:
|
||||
diff_changes = analyze_single_git_diff(diff)
|
||||
changes += [change.bind_to_file(diff.b_path) for change in diff_changes]
|
||||
file_pre, file_post = analyze_single_git_diff(diff)
|
||||
pre += [change.bind_to_file(diff.a_path) for change in file_pre]
|
||||
post += [change.bind_to_file(diff.b_path) for change in file_post]
|
||||
|
||||
return changes
|
||||
return pre, post
|
||||
|
||||
|
||||
def make_check_result(changes: list[FileBoundNoexceptChange]) -> dict[str, Any]:
|
||||
annotations = [change.make_annotation() for change in changes]
|
||||
def analyze_changes(pre: list[FileBoundSnippet], post: list[FileBoundSnippet]) -> list[Alert]:
|
||||
"""
|
||||
Given pre- and post-change snippets, analyze the, and return any alerts that should be raised.
|
||||
"""
|
||||
alerts = []
|
||||
|
||||
# First, check for non-noexcept moves being added (including the case where we remove noexcept
|
||||
# from an existing noexcept move).
|
||||
non_noexcept_moves_post = [s for s in post if s.kind == SnippetKind.NonNoexceptMove]
|
||||
non_noexcept_move_identifiers_pre = {
|
||||
s.identifier for s in pre if s.kind == SnippetKind.NonNoexceptMove
|
||||
}
|
||||
for s in non_noexcept_moves_post:
|
||||
if s.identifier in non_noexcept_move_identifiers_pre:
|
||||
# This was already a non-noexcept move, it's not the PR's fault.
|
||||
LOGGER.info("Skipping alert for existing non-noexcept move", identifier=s.identifier)
|
||||
continue
|
||||
# This is a new non-noexcept move (or noexcept was removed from an existing move) - alert.
|
||||
LOGGER.info("Alerting for non-noexcept move", identifier=s.identifier)
|
||||
alerts.append(
|
||||
Alert(
|
||||
file=s.file,
|
||||
line=s.alert_line,
|
||||
kind=AlertKind.NonNoexceptMove,
|
||||
identifier=s.identifier,
|
||||
)
|
||||
)
|
||||
|
||||
# Ignore any move operations, noexcept or not. We've already checked for the bad case related
|
||||
# to moves, so now we just need to look at other non-move noexcept changes.
|
||||
non_move_pre = [s for s in pre if not s.is_move()]
|
||||
non_move_post = [s for s in post if not s.is_move()]
|
||||
|
||||
# Now we should be left with snippets for non-move noexcept changes only. First, check for the
|
||||
# case where we can definitively detect adding or removing noexcept from existing code.
|
||||
alerted_for_noexcept_change = False
|
||||
for s in non_move_pre + non_move_post:
|
||||
if s.kind in (SnippetKind.NoexceptAddition, SnippetKind.NoexceptRemoval):
|
||||
if alerted_for_noexcept_change:
|
||||
LOGGER.info(
|
||||
"Not alerting for definitive noexcept change (already alerted once)",
|
||||
file=s.file,
|
||||
line=s.alert_line,
|
||||
)
|
||||
else:
|
||||
LOGGER.info(
|
||||
"Alerting for defintive noexcept change", file=s.file, line=s.alert_line
|
||||
)
|
||||
alerts.append(
|
||||
Alert(
|
||||
file=s.file,
|
||||
line=s.alert_line,
|
||||
kind=AlertKind.Noexcept,
|
||||
identifier=s.identifier,
|
||||
)
|
||||
)
|
||||
alerted_for_noexcept_change = True
|
||||
|
||||
# If the count increased, then we're adding new noexcept usage somewhere (either in new code or
|
||||
# in making old code noexcept). We may not have been able to detect these as definitive
|
||||
# additions above. This could happen if e.g., code is both moved to a new file and made
|
||||
# noexcept, or if new code is written using noexcept. If the count descreased, we might just be
|
||||
# deleting code that happens to contain noexcept, so we won't alert in that case.
|
||||
if not alerted_for_noexcept_change and len(non_move_post) > len(non_move_pre):
|
||||
if alerted_for_noexcept_change:
|
||||
LOGGER.info("Not alerting for new noexcept usage (already alerted once)")
|
||||
else:
|
||||
LOGGER.info("Alerting for net new noexcept usage")
|
||||
alerts.append(
|
||||
Alert(
|
||||
file=non_move_post[0].file,
|
||||
line=non_move_post[0].alert_line,
|
||||
kind=AlertKind.Noexcept,
|
||||
identifier=s.identifier,
|
||||
)
|
||||
)
|
||||
alerted_for_noexcept_change = True
|
||||
|
||||
return alerts
|
||||
|
||||
|
||||
def make_check_result(alerts: list[Alert]) -> dict[str, Any]:
|
||||
annotations = [alert.make_annotation() for alert in alerts]
|
||||
return {
|
||||
"title": TITLE_TEXT,
|
||||
"summary": SUMMARY_TEXT,
|
||||
|
|
@ -314,28 +548,9 @@ def make_check_result(changes: list[FileBoundNoexceptChange]) -> dict[str, Any]:
|
|||
|
||||
|
||||
def check_for_noexcept_changes(diffs: list[git.Diff]) -> dict[str, Any]:
|
||||
changes = analyze_git_diff(diffs)
|
||||
|
||||
# If we're certain that noexcept was added or removed in some places, make annotations based
|
||||
# on the changes we're sure of.
|
||||
certain_changes = [change for change in changes if change.is_certain]
|
||||
if len(certain_changes) > 0:
|
||||
return make_check_result(certain_changes)
|
||||
|
||||
# We've detected changes involving noexcept, but we're not certain that they're meaningful.
|
||||
# For example, removing a noexcept function altogether would be picked up as a noexcept
|
||||
# deletion, but it's not one we want to report on.
|
||||
# Conservatively, we'll only report on uncertain noexcept changes if there are additions of
|
||||
# noexcept with _no_ removals. This is still not perfect - we might pick up comments, or
|
||||
# someone declaring a noexcept function that already has a definition elsewhere, for example.
|
||||
additions = [change for change in changes if change.kind == ChangeKind.ADDITION]
|
||||
removals = [change for change in changes if change.kind == ChangeKind.REMOVAL]
|
||||
if len(additions) > 0 and len(removals) == 0:
|
||||
return make_check_result(additions)
|
||||
|
||||
# There are either no changes to noexcept or a mix of added and removed noexcepts. We'd need a
|
||||
# deeper analysis to figure out the mixed case.
|
||||
return make_check_result([])
|
||||
pre, post = analyze_git_diff(diffs)
|
||||
alerts = analyze_changes(pre, post)
|
||||
return make_check_result(alerts)
|
||||
|
||||
|
||||
def main(
|
||||
|
|
@ -348,9 +563,6 @@ def main(
|
|||
output_path: Annotated[
|
||||
str, typer.Option("--output", "-o", help="Path at which to output github annotations")
|
||||
] = "../github_annotations.json",
|
||||
max_annotations: Annotated[
|
||||
int, typer.Option("--limit", help="Maximum number of annotations to output")
|
||||
] = 1,
|
||||
):
|
||||
"""
|
||||
Check for changes in noexcept usage between the given revisions and output a file that
|
||||
|
|
@ -373,12 +585,6 @@ def main(
|
|||
|
||||
check_result = check_for_noexcept_changes(diffs)
|
||||
|
||||
# Cap annotations to the given limit - we only want to output one in the commit queue, but
|
||||
# for testing it's useful to see all of them.
|
||||
if "annotations" in check_result:
|
||||
annotations = check_result.get("annotations", [])
|
||||
if max_annotations > 0 and len(annotations) > max_annotations:
|
||||
check_result["annotations"] = annotations[:max_annotations]
|
||||
with open(output_path, "w") as f:
|
||||
json.dump(check_result, f, indent=4)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,47 +1,49 @@
|
|||
import unittest
|
||||
|
||||
from buildscripts.check_for_noexcept import (
|
||||
ChangeKind,
|
||||
AlertKind,
|
||||
FileBoundSnippet,
|
||||
SnippetKind,
|
||||
analyze_changes,
|
||||
analyze_text_diff,
|
||||
get_matching_blocks,
|
||||
get_move_operations,
|
||||
get_noexcepts,
|
||||
get_nonmatching_blocks,
|
||||
strip_noexcept,
|
||||
)
|
||||
|
||||
|
||||
def _remove_whitespace(s: str) -> str:
|
||||
return "".join(s.split())
|
||||
|
||||
|
||||
class TestGetMatchingBlocks(unittest.TestCase):
|
||||
def _check_blocks_cover_string(self, blocks: list[tuple[int, int]], s: str):
|
||||
lines = s.splitlines()
|
||||
if len(blocks) == 0:
|
||||
self.assertEqual(len(s), 0)
|
||||
self.assertEqual(len(lines), 0)
|
||||
return
|
||||
|
||||
prev_end = 0
|
||||
for block_start, block_end in sorted(blocks):
|
||||
self.assertEqual(block_start, prev_end)
|
||||
prev_end = block_end
|
||||
self.assertEqual(prev_end, len(s))
|
||||
self.assertEqual(prev_end, len(lines))
|
||||
|
||||
def test_matches_and_nonmatches_are_accurate(self):
|
||||
cases = (
|
||||
("const noexcept try {", "const try {"),
|
||||
("a b c d", "a b c d e"),
|
||||
("b c d e", "a b c d e"),
|
||||
("int f(SomeType<float> x) noexcept const {", "int f(SomeType<float> x) const {"),
|
||||
("int f(SomeType<float> x) noexcept const\n{", "int\nf(SomeType<float> x) const {"),
|
||||
("abc\nbcd\ncde\ndef", "abc\nwxy\ncde\ndef"),
|
||||
("abc\nbcd\ncde\ndef", "abc\ncde\ndef"),
|
||||
("abc\nbcd\ncde\ndef", "abc\nbcd\ncde\ndef\nxyz"),
|
||||
)
|
||||
for lhs, rhs in cases:
|
||||
matching = get_matching_blocks(lhs, rhs)
|
||||
nonmatching = get_nonmatching_blocks(lhs, rhs)
|
||||
self.assertGreater(len(matching), 0)
|
||||
self.assertGreater(len(nonmatching), 0)
|
||||
|
||||
lhs_lines, rhs_lines = lhs.splitlines(), rhs.splitlines()
|
||||
for lhs_start, lhs_end, rhs_start, rhs_end in matching:
|
||||
block_lhs = lhs[lhs_start:lhs_end]
|
||||
block_rhs = rhs[rhs_start:rhs_end]
|
||||
# Note that the matching ignores whitespace, so "matching" sequences may
|
||||
# differ in whitespace.
|
||||
self.assertEqual(_remove_whitespace(block_lhs), _remove_whitespace(block_rhs))
|
||||
block_lhs = "\n".join(lhs_lines[lhs_start:lhs_end])
|
||||
block_rhs = "\n".join(rhs_lines[rhs_start:rhs_end])
|
||||
self.assertEqual(block_lhs, block_rhs)
|
||||
lhs_blocks = []
|
||||
rhs_blocks = []
|
||||
for lhs_start, lhs_end, rhs_start, rhs_end in matching + nonmatching:
|
||||
|
|
@ -55,37 +57,226 @@ class TestGetMatchingBlocks(unittest.TestCase):
|
|||
self.assertEqual(get_matching_blocks("nonempty", ""), [])
|
||||
self.assertEqual(get_matching_blocks("", ""), [])
|
||||
|
||||
self.assertEqual(get_nonmatching_blocks("", "nonempty"), [(0, 0, 0, 1)])
|
||||
self.assertEqual(get_nonmatching_blocks("nonempty", ""), [(0, 1, 0, 0)])
|
||||
self.assertEqual(get_nonmatching_blocks("", ""), [])
|
||||
|
||||
def test_no_match(self):
|
||||
self.assertEqual(get_matching_blocks("a b c d", "e f g h"), [])
|
||||
self.assertEqual(get_nonmatching_blocks("a b c d", "e f g h"), [(0, 1, 0, 1)])
|
||||
|
||||
def test_equal_strings(self):
|
||||
s = "a b c d"
|
||||
self.assertEqual(get_matching_blocks(s, s), [(0, len(s), 0, len(s))])
|
||||
self.assertEqual(get_matching_blocks(s, s), [(0, 1, 0, 1)])
|
||||
self.assertEqual(get_nonmatching_blocks(s, s), [])
|
||||
|
||||
def test_basic_multiline_edit(self):
|
||||
lhs = "a\nb\nc\nd"
|
||||
rhs = "a\nw\nx\nd"
|
||||
self.assertEqual(get_matching_blocks(lhs, rhs), [(0, 1, 0, 1), (3, 4, 3, 4)])
|
||||
self.assertEqual(get_nonmatching_blocks(lhs, rhs), [(1, 3, 1, 3)])
|
||||
|
||||
def test_basic_multiline_delete(self):
|
||||
lhs = "a\nb\nc\nd"
|
||||
rhs = "a\nd"
|
||||
self.assertEqual(get_matching_blocks(lhs, rhs), [(0, 1, 0, 1), (3, 4, 1, 2)])
|
||||
self.assertEqual(get_nonmatching_blocks(lhs, rhs), [(1, 3, 1, 1)])
|
||||
|
||||
def test_basic_multiline_insert(self):
|
||||
lhs = "a\nd"
|
||||
rhs = "a\nb\nc\nd"
|
||||
self.assertEqual(get_matching_blocks(lhs, rhs), [(0, 1, 0, 1), (1, 2, 3, 4)])
|
||||
self.assertEqual(get_nonmatching_blocks(lhs, rhs), [(1, 1, 1, 3)])
|
||||
|
||||
|
||||
# The basic cases below end with semicolons (generally). We can generate similar cases ending with {
|
||||
# or just without the semicolon to cover those cases.
|
||||
def _generate_additional_cases(cases: list[str]) -> tuple[str]:
|
||||
new_cases = list(cases)
|
||||
for case in cases:
|
||||
if case.endswith(";"):
|
||||
new_cases.append(case[:-1] + " {")
|
||||
new_cases.append(case[:-1])
|
||||
return tuple(set(new_cases))
|
||||
|
||||
|
||||
# The hardcoded cases all contain noexcept. We can generate non-noexcept cases by removing any
|
||||
# noexcept tokens.
|
||||
def _generate_non_noexcept_cases(cases: list[str]) -> tuple[str]:
|
||||
new_cases = [strip_noexcept(case) for case in cases]
|
||||
return tuple(set(new_cases))
|
||||
|
||||
|
||||
_BASIC_NOEXCEPT_MOVE_CONSTRUCTORS = _generate_additional_cases(
|
||||
[
|
||||
"TypeName(TypeName&& other) const noexcept;",
|
||||
"TypeName(TypeName&& other) const noexcept(std::is_nothrow_constructible_v<TypeName>);",
|
||||
"TypeName(TypeName&& other) noexcept;",
|
||||
"TypeName(TypeName&& other) noexcept const;",
|
||||
"TypeName(TypeName&& other) noexcept(std::is_nothrow_constructible_v<TypeName>) const;",
|
||||
"Typenoexcept(Typenoexcept&& other) const noexcept;",
|
||||
"Typenoexcept(Typenoexcept&& other) noexcept;",
|
||||
"Typenoexcept(Typenoexcept&& other) noexcept const;",
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
_BASIC_NOEXCEPT_MOVE_ASSIGNMENTS = _generate_additional_cases(
|
||||
[
|
||||
"TypeName& operator=(TypeName&&) noexcept;",
|
||||
"TypeName& operator=(TypeName&&) noexcept(std::is_nothrow_constructible_v<TypeName>);",
|
||||
"TypeName& operator=(TypeName&&) noexcept { return *this; }",
|
||||
"TypeName& operator=(TypeName&&) noexcept const;",
|
||||
"TypeName& operator=(TypeName&&) noexcept const { return *this; }",
|
||||
"TypeName& operator=(TypeName&&) noexcept(std::is_nothrow_constructible_v<TypeName>) const { return *this; }",
|
||||
"TypeName& operator=(TypeName&&) const noexcept;",
|
||||
"TypeName& operator=(TypeName&&) const noexcept { return *this; }",
|
||||
"Typenoexcept& operator=(Typenoexcept&&) noexcept { return *this; }",
|
||||
"Typenoexcept& operator=(Typenoexcept&&) noexcept(std::is_nothrow_constructible_v<Typenoexcept>) { return *this; }",
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
_BASIC_NON_NOEXCEPT_MOVE_CONSTRUCTORS = _generate_non_noexcept_cases(
|
||||
_BASIC_NOEXCEPT_MOVE_CONSTRUCTORS
|
||||
)
|
||||
|
||||
|
||||
_BASIC_NON_NOEXCEPT_MOVE_ASSIGNMENTS = _generate_non_noexcept_cases(
|
||||
_BASIC_NOEXCEPT_MOVE_ASSIGNMENTS
|
||||
)
|
||||
|
||||
|
||||
_BASIC_NOEXCEPT_MOVE_OPERATIONS = (
|
||||
_BASIC_NOEXCEPT_MOVE_CONSTRUCTORS + _BASIC_NOEXCEPT_MOVE_ASSIGNMENTS
|
||||
)
|
||||
|
||||
|
||||
_BASIC_NON_NOEXCEPT_MOVE_OPERATIONS = (
|
||||
_BASIC_NON_NOEXCEPT_MOVE_CONSTRUCTORS + _BASIC_NON_NOEXCEPT_MOVE_ASSIGNMENTS
|
||||
)
|
||||
|
||||
|
||||
# Tricky statements that look similar-ish to move operations but are not.
|
||||
_BASIC_NOEXCEPT_NON_MOVE_OPERATIONS = _generate_additional_cases(
|
||||
[
|
||||
"functionName(TypeName&&) noexcept;",
|
||||
"functionName(TypeName&& other) noexcept;",
|
||||
"functionName(TypeName&&) const noexcept;",
|
||||
"functionName(TypeName&&, int) const noexcept;",
|
||||
"functionName(TypeName&& other, int otherParam) noexcept;",
|
||||
"functionName() noexcept;",
|
||||
"TypeName() noexcept;",
|
||||
"TypeName(const TypeName&) noexcept;",
|
||||
"TypeName(const TypeName&) noexcept(std::is_trivially_constructible_v<TypeName>);",
|
||||
"TypeName(TypeName) noexcept;",
|
||||
"functionName(TypeName val) noexcept;",
|
||||
"functionName(TypeName& val) noexcept;",
|
||||
"functionName(TypeName&& val) noexcept;",
|
||||
"auto lambda = [x = std::move(y)]() noexcept { return x; }",
|
||||
"auto lambda = [](TypeName&& x) noexcept { return x; }",
|
||||
"[](TypeName&& x) noexcept { return x; }",
|
||||
"([](TypeName&& x) noexcept { return x; })()",
|
||||
"TypeName(TypeName&&, int otherParam) noexcept;",
|
||||
"TypeName operator+(const TypeName& other) noexcept;",
|
||||
"TypeName operator+(const TypeName& other) const noexcept;",
|
||||
"TypeName& operator*() const noexcept;",
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
_BASIC_NON_NOEXCEPT_NON_MOVE_OPERATIONS = _generate_non_noexcept_cases(
|
||||
_BASIC_NOEXCEPT_NON_MOVE_OPERATIONS
|
||||
)
|
||||
|
||||
|
||||
def _noexcept_moves_only(snippets):
|
||||
return [s for s in snippets if s.kind == SnippetKind.NoexceptMove]
|
||||
|
||||
|
||||
def _nonnoexcept_moves_only(snippets):
|
||||
return [s for s in snippets if s.kind == SnippetKind.NonNoexceptMove]
|
||||
|
||||
|
||||
def _make_snippet(
|
||||
*,
|
||||
file: str,
|
||||
kind: SnippetKind,
|
||||
identifier: str | None = None,
|
||||
line: int | None = None,
|
||||
line_start: int | None = None,
|
||||
line_end: int | None = None,
|
||||
alert_line: int | None = None,
|
||||
):
|
||||
if line is not None:
|
||||
line_start = line
|
||||
line_end = line
|
||||
else:
|
||||
line_start = line_start
|
||||
line_end = line_end
|
||||
|
||||
if alert_line is None:
|
||||
alert_line = line_start
|
||||
|
||||
return FileBoundSnippet(
|
||||
file=file,
|
||||
kind=kind,
|
||||
start=0,
|
||||
end=0,
|
||||
identifier=identifier,
|
||||
line_start=line_start,
|
||||
line_end=line_end,
|
||||
alert_line=alert_line,
|
||||
)
|
||||
|
||||
|
||||
class TestCheckForNoexcept(unittest.TestCase):
|
||||
def test_no_text(self):
|
||||
changes = analyze_text_diff("", "")
|
||||
self.assertEqual(changes, [])
|
||||
def test_strip_noexcept(self):
|
||||
no_noexcept_cases = _BASIC_NON_NOEXCEPT_MOVE_OPERATIONS
|
||||
for case in no_noexcept_cases:
|
||||
self.assertEqual(strip_noexcept(case), case)
|
||||
|
||||
def test_simple_additions(self):
|
||||
self.assertEqual(
|
||||
strip_noexcept(
|
||||
"int f(SomeType<float> x) noexcept(std::is_nothrow_constructible_v<SomeType<float>>);"
|
||||
),
|
||||
"int f(SomeType<float> x);",
|
||||
)
|
||||
self.assertEqual(
|
||||
strip_noexcept("int f(SomeType<float> x) noexcept const;"),
|
||||
"int f(SomeType<float> x) const;",
|
||||
)
|
||||
self.assertEqual(
|
||||
strip_noexcept("int f(SomeType<float> x) noexcept;"),
|
||||
"int f(SomeType<float> x);",
|
||||
)
|
||||
|
||||
def test_no_text(self):
|
||||
pre, post = analyze_text_diff("", "")
|
||||
self.assertEqual(pre, [])
|
||||
self.assertEqual(post, [])
|
||||
|
||||
def test_simple_noexcept_additions(self):
|
||||
pre = "int f(SomeType<float> x) const {"
|
||||
posts = (
|
||||
"int f(SomeType<float> x) noexcept const {",
|
||||
"int f(SomeType<float> x) noexcept {",
|
||||
"int f(SomeType<float> x) noexcept(std::is_nothrow_constructible_v<SomeType<float>>) {",
|
||||
"int func(SomeType<float> x) noexcept const {",
|
||||
"int f(SomeType<double> x) noexcept const {",
|
||||
"int f(SomeType<float> y) noexcept const {",
|
||||
"int f(SomeOtherType<float> x) noexcept const {",
|
||||
)
|
||||
for post in posts:
|
||||
changes = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(changes), 1)
|
||||
change = changes[0]
|
||||
self.assertEqual(change.kind, ChangeKind.ADDITION)
|
||||
self.assertTrue(change.is_certain)
|
||||
self.assertEqual(change.index, post.find("noexcept"))
|
||||
self.assertEqual(change.line, 1)
|
||||
before, after = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(before), 0)
|
||||
self.assertEqual(len(after), 1)
|
||||
snippet = after[0]
|
||||
self.assertFalse(snippet.is_move())
|
||||
self.assertEqual(snippet.start, post.find("noexcept"))
|
||||
self.assertEqual(snippet.end, snippet.start + len("noexcept"))
|
||||
self.assertEqual(snippet.line_start, 1)
|
||||
self.assertEqual(snippet.line_end, 1)
|
||||
|
||||
def test_simple_removals(self):
|
||||
pre = "int f(SomeType<float> x) noexcept const {"
|
||||
|
|
@ -98,19 +289,61 @@ class TestCheckForNoexcept(unittest.TestCase):
|
|||
"int f(SomeOtherType<float> x) const {",
|
||||
)
|
||||
for post in posts:
|
||||
changes = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(changes), 1)
|
||||
change = changes[0]
|
||||
self.assertEqual(change.kind, ChangeKind.REMOVAL)
|
||||
self.assertTrue(change.is_certain)
|
||||
self.assertEqual(change.index, pre.find("noexcept"))
|
||||
self.assertEqual(change.line, 1)
|
||||
before, after = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(before), 1)
|
||||
self.assertEqual(len(after), 0)
|
||||
snippet = before[0]
|
||||
self.assertFalse(snippet.is_move())
|
||||
self.assertEqual(snippet.start, pre.find("noexcept"))
|
||||
self.assertEqual(snippet.end, snippet.start + len("noexcept"))
|
||||
self.assertEqual(snippet.line_start, 1)
|
||||
self.assertEqual(snippet.line_end, 1)
|
||||
|
||||
def test_simple_unchanged(self):
|
||||
def test_definite_noexcept_addition_removal(self):
|
||||
with_noexcept = """
|
||||
int f() {
|
||||
return x * y;
|
||||
}
|
||||
int g(int z) const noexcept(std::is_nothrow_constructible_v<T>) {
|
||||
return z + 1;
|
||||
}
|
||||
void q() noexcept {
|
||||
// do nothing
|
||||
}
|
||||
"""
|
||||
without_noexcept = """
|
||||
int f() {
|
||||
return x * y;
|
||||
}
|
||||
int g(int z) const {
|
||||
return z + 1;
|
||||
}
|
||||
void q() {
|
||||
// do nothing
|
||||
}
|
||||
"""
|
||||
# Case where noexcept was definitively removed.
|
||||
before, after = analyze_text_diff(with_noexcept, without_noexcept)
|
||||
self.assertEqual(len(before), 2)
|
||||
self.assertEqual(len(after), 0)
|
||||
for snippet in before:
|
||||
self.assertEqual(snippet.kind, SnippetKind.NoexceptRemoval)
|
||||
|
||||
# Case where noexcept was definitively added.
|
||||
before, after = analyze_text_diff(without_noexcept, with_noexcept)
|
||||
self.assertEqual(len(before), 0)
|
||||
self.assertEqual(len(after), 2)
|
||||
for snippet in before:
|
||||
self.assertEqual(snippet.kind, SnippetKind.NoexceptRemoval)
|
||||
|
||||
def test_simple_unchanged_with_noexcept(self):
|
||||
pre = "int f(SomeType<float> x) noexcept {"
|
||||
post = "int f(SomeType<float> x) noexcept {"
|
||||
changes = analyze_text_diff(pre, post)
|
||||
self.assertEqual(changes, [])
|
||||
post = "float f(SomeOtherType<int> y) noexcept {"
|
||||
before, after = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(before), 1)
|
||||
self.assertEqual(before[0].kind, SnippetKind.Noexcept)
|
||||
self.assertEqual(len(after), 1)
|
||||
self.assertEqual(after[0].kind, SnippetKind.Noexcept)
|
||||
|
||||
def test_simple_unrelated_changes(self):
|
||||
pre = "int f(SomeType<float> x) noexcept const {"
|
||||
|
|
@ -122,70 +355,362 @@ class TestCheckForNoexcept(unittest.TestCase):
|
|||
"int f(SomeOtherType<float> x) noexcept const {",
|
||||
)
|
||||
for post in posts:
|
||||
changes = analyze_text_diff(pre, post)
|
||||
self.assertEqual(changes, [])
|
||||
before, after = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(before), 1)
|
||||
self.assertEqual(before[0].kind, SnippetKind.Noexcept)
|
||||
self.assertEqual(len(after), 1)
|
||||
self.assertEqual(after[0].kind, SnippetKind.Noexcept)
|
||||
|
||||
def test_complex_change(self):
|
||||
pre = """
|
||||
class SomeClass {
|
||||
template<typename T>
|
||||
auto memberFunction(std::unique_ptr<T> x) const {
|
||||
thisFunctionCanThrow();
|
||||
return x.get();
|
||||
}
|
||||
void otherMemberFunction() const noexcept try {
|
||||
thisFunctionCanThrow()
|
||||
} catch (DBException& e) {
|
||||
}
|
||||
};
|
||||
"""
|
||||
post = """
|
||||
class SomeClass {
|
||||
template<typename T>
|
||||
int memberFunctionNoThrow(std::unique_ptr<T> x) const noexcept {
|
||||
return x.get();
|
||||
}
|
||||
void otherMemberFunction() const {
|
||||
thisFunctionCanThrow()
|
||||
}
|
||||
};
|
||||
"""
|
||||
changes = analyze_text_diff(pre, post)
|
||||
additions = [change for change in changes if change.kind == ChangeKind.ADDITION]
|
||||
removals = [change for change in changes if change.kind == ChangeKind.REMOVAL]
|
||||
self.assertEqual(len(additions), 1)
|
||||
self.assertEqual(len(removals), 1)
|
||||
addition = additions[0]
|
||||
removal = removals[0]
|
||||
# Note that lines are all relative to the "post" side and the "SomeClass" is line 2
|
||||
# (lines are 1-indexed and line 1 is a blank line).
|
||||
self.assertEqual(addition.line, 4)
|
||||
self.assertEqual(removal.line, 7)
|
||||
def test_simple_noexcept_move_addition(self):
|
||||
pre = "\n\nTypeName(TypeName&& other);"
|
||||
post = "\n\nTypeName(TypeName&& other)\nnoexcept;"
|
||||
before, after = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(before), 1)
|
||||
snippet = before[0]
|
||||
self.assertEqual(snippet.kind, SnippetKind.NonNoexceptMove)
|
||||
self.assertEqual(snippet.start, 2)
|
||||
self.assertEqual(snippet.end, pre.find(")") + 1)
|
||||
self.assertEqual(snippet.line_start, 3)
|
||||
self.assertEqual(snippet.line_end, 3)
|
||||
|
||||
def test_rewrite_change(self):
|
||||
pre = """
|
||||
class SomeClass {
|
||||
auto memberFunction(std::unique_ptr<int> x) const { return x.get(); }
|
||||
void otherMemberFunction() const { return 7; }
|
||||
template<typename T>
|
||||
T anotherMemberFunction(T x) const { return x; }
|
||||
};
|
||||
"""
|
||||
post = """
|
||||
class SomeClass {
|
||||
template<typename T>
|
||||
void anotherNewNoexceptFunction() noexcept { doSomething(); }
|
||||
auto someNewNoexceptFunction(std::shared_ptr<T> x) const noexcept { return *x; }
|
||||
int anotherNewNoexceptFunction2() const noexcept { return 7; }
|
||||
};
|
||||
"""
|
||||
changes = analyze_text_diff(pre, post)
|
||||
additions = [change for change in changes if change.kind == ChangeKind.ADDITION]
|
||||
removals = [change for change in changes if change.kind == ChangeKind.REMOVAL]
|
||||
self.assertEqual(len(additions), 3)
|
||||
self.assertEqual(len(removals), 0)
|
||||
lines = sorted([addition.line for addition in additions])
|
||||
self.assertEqual(lines, [4, 5, 6])
|
||||
self.assertEqual(len(after), 1)
|
||||
snippet = after[0]
|
||||
self.assertEqual(snippet.kind, SnippetKind.NoexceptMove)
|
||||
self.assertEqual(snippet.start, 2)
|
||||
self.assertEqual(snippet.end, len(post) - 1)
|
||||
self.assertEqual(snippet.line_start, 3)
|
||||
self.assertEqual(snippet.line_end, 4)
|
||||
|
||||
def test_simple_noexcept_move_removal(self):
|
||||
pre = "\n\nTypeName(TypeName&& other) noexcept {\n /* code */ }"
|
||||
post = "\n\nTypeName(TypeName&& other) {\n /* code */ }"
|
||||
before, after = analyze_text_diff(pre, post)
|
||||
self.assertEqual(len(before), 1)
|
||||
snippet = before[0]
|
||||
self.assertEqual(snippet.kind, SnippetKind.NoexceptMove)
|
||||
self.assertEqual(snippet.start, 2)
|
||||
self.assertEqual(snippet.end, pre.find("{"))
|
||||
self.assertEqual(snippet.line_start, 3)
|
||||
self.assertEqual(snippet.line_end, 3)
|
||||
|
||||
self.assertEqual(len(after), 1)
|
||||
snippet = after[0]
|
||||
self.assertEqual(snippet.kind, SnippetKind.NonNoexceptMove)
|
||||
self.assertEqual(snippet.start, 2)
|
||||
self.assertEqual(snippet.end, post.find(")") + 1)
|
||||
self.assertEqual(snippet.line_start, 3)
|
||||
self.assertEqual(snippet.line_end, 3)
|
||||
|
||||
def test_get_move_operations_noexcept_independent(self):
|
||||
# Basic cases
|
||||
with_move_operation = _BASIC_NOEXCEPT_MOVE_OPERATIONS + _BASIC_NON_NOEXCEPT_MOVE_OPERATIONS
|
||||
without_move_operation = (
|
||||
_BASIC_NOEXCEPT_NON_MOVE_OPERATIONS + _BASIC_NON_NOEXCEPT_NON_MOVE_OPERATIONS
|
||||
)
|
||||
|
||||
# Trickier cases
|
||||
with_move_operation += (
|
||||
"""
|
||||
class TypeName {
|
||||
TypeName(TypeName&&) noexcept;
|
||||
};
|
||||
""",
|
||||
"""
|
||||
TypeName& operator=(TypeName&& other) noexcept {
|
||||
auto lambda = [x = std::move(y)]() noexcept { return x; };",
|
||||
z = lambda();
|
||||
return *this;
|
||||
}
|
||||
""",
|
||||
"TypeName(TypeName&&) { /* noexcept */ }",
|
||||
"TypeName(TypeName&&); // noexcept",
|
||||
"Typenoexcept(Typenoexcept&& varnoexcept);",
|
||||
"TypeName(TypeName&&); TypeName() noexcept;",
|
||||
"Typenoexcept& operator=(Typenoexcept&&) { return *this; }",
|
||||
"""
|
||||
TypeName& operator=(TypeName&& other) {
|
||||
auto lambda = [x = std::move(y)]() noexcept { return x; };",
|
||||
z = lambda();
|
||||
return *this;
|
||||
}
|
||||
""",
|
||||
)
|
||||
|
||||
without_move_operation += (
|
||||
"""
|
||||
class TypeName {
|
||||
TypeName(const TypeName&);
|
||||
TypeName(TypeName);
|
||||
functionName(TypeName&&);
|
||||
};
|
||||
""",
|
||||
)
|
||||
|
||||
for snippet in with_move_operation:
|
||||
all_moves = get_move_operations(snippet)
|
||||
self.assertEqual(len(all_moves), 1, msg=f"Expected move operation: {snippet}")
|
||||
|
||||
for snippet in without_move_operation:
|
||||
all_moves = get_move_operations(snippet)
|
||||
self.assertEqual(len(all_moves), 0, msg=f"Expected no move operation in {snippet}")
|
||||
|
||||
def test_get_move_operations_noexcept_matters(self):
|
||||
# Basic cases
|
||||
with_noexcept = _BASIC_NOEXCEPT_MOVE_OPERATIONS
|
||||
without_noexcept = _BASIC_NON_NOEXCEPT_MOVE_OPERATIONS
|
||||
|
||||
# Trickier cases
|
||||
with_noexcept += (
|
||||
"Typenoexcept& operator=(Typenoexcept&&) noexcept { return *this; }",
|
||||
"""
|
||||
TypeName& operator=(TypeName&& other) noexcept {
|
||||
auto lambda = [x = std::move(y)]() noexcept { return x; };",
|
||||
z = lambda();
|
||||
return *this;
|
||||
}
|
||||
""",
|
||||
)
|
||||
|
||||
without_noexcept += (
|
||||
"TypeName(TypeName&&) { /* noexcept */ }",
|
||||
"TypeName(TypeName&&); // noexcept",
|
||||
"Typenoexcept(Typenoexcept&& varnoexcept);",
|
||||
"TypeName(TypeName&&); TypeName() noexcept;",
|
||||
"Typenoexcept& operator=(Typenoexcept&&) { return *this; }",
|
||||
"""
|
||||
TypeName& operator=(TypeName&& other) {
|
||||
auto lambda = [x = std::move(y)]() noexcept { return x; };",
|
||||
z = lambda();
|
||||
return *this;
|
||||
}
|
||||
""",
|
||||
)
|
||||
|
||||
for snippet in with_noexcept:
|
||||
all_moves = get_move_operations(snippet)
|
||||
noexcept = _noexcept_moves_only(all_moves)
|
||||
non_noexcept = _nonnoexcept_moves_only(all_moves)
|
||||
self.assertEqual(len(noexcept), 1, msg=f"Expected noexcept move operation: {snippet}")
|
||||
self.assertEqual(
|
||||
len(non_noexcept), 0, msg=f"Expected no non-noexcept move operation: {snippet}"
|
||||
)
|
||||
|
||||
for snippet in without_noexcept:
|
||||
all_moves = get_move_operations(snippet)
|
||||
noexcept = _noexcept_moves_only(all_moves)
|
||||
non_noexcept = _nonnoexcept_moves_only(all_moves)
|
||||
self.assertEqual(
|
||||
len(noexcept), 0, msg=f"Expected no noexcept move operation: {snippet}"
|
||||
)
|
||||
self.assertEqual(
|
||||
len(non_noexcept), 1, msg=f"Expected non-noexcept move operation: {snippet}"
|
||||
)
|
||||
|
||||
def test_get_move_operations_negative_cases(self):
|
||||
negative_cases = (
|
||||
_BASIC_NOEXCEPT_NON_MOVE_OPERATIONS + _BASIC_NON_NOEXCEPT_NON_MOVE_OPERATIONS
|
||||
)
|
||||
|
||||
for snippet in negative_cases:
|
||||
all_moves = get_move_operations(snippet)
|
||||
self.assertEqual(len(all_moves), 0, msg=f"Expected no move operation: {snippet}")
|
||||
|
||||
def test_get_noexcepts(self):
|
||||
with_noexcept = _BASIC_NOEXCEPT_MOVE_OPERATIONS + _BASIC_NOEXCEPT_NON_MOVE_OPERATIONS
|
||||
without_noexcept = (
|
||||
_BASIC_NON_NOEXCEPT_MOVE_OPERATIONS + _BASIC_NON_NOEXCEPT_NON_MOVE_OPERATIONS
|
||||
)
|
||||
|
||||
for snippet in with_noexcept:
|
||||
self.assertGreater(
|
||||
len(get_noexcepts(snippet)), 0, msg=f"Expected noexcept in snippet: {snippet}"
|
||||
)
|
||||
|
||||
for snippet in without_noexcept:
|
||||
self.assertEqual(
|
||||
len(get_noexcepts(snippet)), 0, msg=f"Expected no noexcept in snippet: {snippet}"
|
||||
)
|
||||
|
||||
def test_move_identifiers(self):
|
||||
templates = (
|
||||
"{t}({t}&&){c}{n};",
|
||||
"{t}({t}&& {v}){c}{n};",
|
||||
"operator=({t}&&){c}{n};",
|
||||
"operator=({t}&& {v}){c}{n};",
|
||||
"{t}& operator=({t}&&){c}{n};",
|
||||
"{t}& operator=({t}&& {v}){c}{n};",
|
||||
"{t}({t}&&)\n{c}\n{n};",
|
||||
"operator=({t}&&)\n{c}{n};",
|
||||
)
|
||||
|
||||
for template in templates:
|
||||
for const in (" const", ""):
|
||||
for noexcept in (" noexcept", "noexcept(condition)", ""):
|
||||
for varname in ("other", "varnoexcept", ""):
|
||||
for typename in ("TypeName", "Typenoexcept"):
|
||||
case = template.format(t=typename, v=varname, c=const, n=noexcept)
|
||||
all_moves = get_move_operations(case)
|
||||
self.assertEqual(
|
||||
len(all_moves), 1, msg=f"Expected move operation: {case}"
|
||||
)
|
||||
move = all_moves[0]
|
||||
if "operator=" in template:
|
||||
identifier = typename + "::operator="
|
||||
else:
|
||||
identifier = typename + "::" + typename
|
||||
self.assertEqual(move.identifier, identifier)
|
||||
|
||||
def test_analyze_changes_simple_addition_removal(self):
|
||||
# No alert on noexcept deletion.
|
||||
snippets = [
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line_start=10, line_end=10),
|
||||
]
|
||||
alerts = analyze_changes(snippets, [])
|
||||
self.assertEqual(len(alerts), 0)
|
||||
|
||||
# Alert on noexcept addition.
|
||||
alerts = analyze_changes([], snippets)
|
||||
self.assertEqual(len(alerts), 1)
|
||||
alert = alerts[0]
|
||||
self.assertEqual(alert.file, "file1.cpp")
|
||||
self.assertEqual(alert.kind, AlertKind.Noexcept)
|
||||
self.assertEqual(alert.line, 10)
|
||||
|
||||
def test_analyze_changes_non_noexcept_move(self):
|
||||
other_snippets = [
|
||||
_make_snippet(
|
||||
file="file1.cpp", kind=SnippetKind.NoexceptMove, identifier="irrelevant", line=5
|
||||
),
|
||||
_make_snippet(file="file2.cpp", kind=SnippetKind.Noexcept, line=10),
|
||||
]
|
||||
|
||||
non_noexcept_move_snippet = [
|
||||
_make_snippet(
|
||||
file="file3.cpp",
|
||||
kind=SnippetKind.NonNoexceptMove,
|
||||
identifier="TypeName::TypeName",
|
||||
line=15,
|
||||
),
|
||||
]
|
||||
|
||||
# The non-noexcept move is present on both sides, no alert.
|
||||
alerts = analyze_changes(non_noexcept_move_snippet, non_noexcept_move_snippet)
|
||||
self.assertEqual(len(alerts), 0)
|
||||
alerts = analyze_changes(
|
||||
other_snippets + non_noexcept_move_snippet, other_snippets + non_noexcept_move_snippet
|
||||
)
|
||||
self.assertEqual(len(alerts), 0)
|
||||
|
||||
# Other stuff is changing, but the non-noexcept move is not changing, no alert.
|
||||
alerts = analyze_changes(
|
||||
other_snippets + non_noexcept_move_snippet, non_noexcept_move_snippet
|
||||
)
|
||||
self.assertFalse(any(alert.kind == AlertKind.NonNoexceptMove for alert in alerts))
|
||||
alerts = analyze_changes(
|
||||
non_noexcept_move_snippet, other_snippets + non_noexcept_move_snippet
|
||||
)
|
||||
self.assertFalse(any(alert.kind == AlertKind.NonNoexceptMove for alert in alerts))
|
||||
|
||||
# The non-noexcept move is removed, this is fine, no alert.
|
||||
alerts = analyze_changes(other_snippets + non_noexcept_move_snippet, other_snippets)
|
||||
self.assertEqual(len(alerts), 0)
|
||||
|
||||
# The non-noexcept move is added, this is an alert.
|
||||
alerts = analyze_changes(other_snippets, other_snippets + non_noexcept_move_snippet)
|
||||
self.assertEqual(len(alerts), 1)
|
||||
alert = alerts[0]
|
||||
self.assertEqual(alert.file, "file3.cpp")
|
||||
self.assertEqual(alert.line, 15)
|
||||
self.assertEqual(alert.identifier, "TypeName::TypeName")
|
||||
self.assertEqual(alert.kind, AlertKind.NonNoexceptMove)
|
||||
|
||||
def test_analyze_changes_complex_case(self):
|
||||
# We have a non-noexcept move (TypeName) in both pre and post, so no alert for that.
|
||||
# We have a net addition of noexcept, so alert for that.
|
||||
# We have a new non-noexcept move (OtherType), so alert for that.
|
||||
pre_snippets = [
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line=10),
|
||||
_make_snippet(
|
||||
file="file2.cpp",
|
||||
kind=SnippetKind.NonNoexceptMove,
|
||||
identifier="TypeName::TypeName",
|
||||
line=20,
|
||||
),
|
||||
]
|
||||
post_snippets = [
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line=10),
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line=30),
|
||||
_make_snippet(
|
||||
file="file2.cpp",
|
||||
kind=SnippetKind.NoexceptMove,
|
||||
identifier="TypeName::TypeName",
|
||||
line=20,
|
||||
),
|
||||
_make_snippet(
|
||||
file="file3.cpp",
|
||||
kind=SnippetKind.NonNoexceptMove,
|
||||
identifier="OtherType::OtherType",
|
||||
line=30,
|
||||
),
|
||||
_make_snippet(
|
||||
file="file4.cpp",
|
||||
kind=SnippetKind.NonNoexceptMove,
|
||||
identifier="TypeName::TypeName",
|
||||
line=40,
|
||||
),
|
||||
]
|
||||
|
||||
alerts = analyze_changes(pre_snippets, post_snippets)
|
||||
self.assertEqual(len(alerts), 2)
|
||||
|
||||
noexcept_move_alerts = [
|
||||
alert for alert in alerts if alert.kind == AlertKind.NonNoexceptMove
|
||||
]
|
||||
self.assertEqual(len(noexcept_move_alerts), 1)
|
||||
noexcept_move_alert = noexcept_move_alerts[0]
|
||||
self.assertEqual(noexcept_move_alert.file, "file3.cpp")
|
||||
self.assertEqual(noexcept_move_alert.line, 30)
|
||||
self.assertEqual(noexcept_move_alert.identifier, "OtherType::OtherType")
|
||||
|
||||
noexcept_change_alerts = [alert for alert in alerts if alert.kind == AlertKind.Noexcept]
|
||||
self.assertEqual(len(noexcept_change_alerts), 1)
|
||||
noexcept_change_alert = noexcept_change_alerts[0]
|
||||
self.assertEqual(noexcept_change_alert.file, "file1.cpp")
|
||||
|
||||
def test_analyze_changes_definitive_additions(self):
|
||||
post_snippets = [
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line=10),
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.NoexceptAddition, line=20),
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.NoexceptAddition, line=30),
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line=40),
|
||||
]
|
||||
alerts = analyze_changes([], post_snippets)
|
||||
# Only alert once for a definitive addition, even if there are multiple snippets.
|
||||
# Also don't alert for the net additions of noexcept since it would be repetitive.
|
||||
self.assertEqual(len(alerts), 1)
|
||||
alert = alerts[0]
|
||||
self.assertEqual(alert.file, "file1.cpp")
|
||||
self.assertEqual(alert.kind, AlertKind.Noexcept)
|
||||
self.assertEqual(alert.line, 20)
|
||||
|
||||
def test_analyze_changes_definitive_removals(self):
|
||||
pre_snippets = [
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line=10, alert_line=510),
|
||||
_make_snippet(
|
||||
file="file1.cpp", kind=SnippetKind.NoexceptAddition, line=20, alert_line=520
|
||||
),
|
||||
_make_snippet(
|
||||
file="file1.cpp", kind=SnippetKind.NoexceptAddition, line=30, alert_line=530
|
||||
),
|
||||
_make_snippet(file="file1.cpp", kind=SnippetKind.Noexcept, line=40, alert_line=540),
|
||||
]
|
||||
alerts = analyze_changes(pre_snippets, [])
|
||||
# Only alert once for a definitive removal, even if there are multiple snippets.
|
||||
self.assertEqual(len(alerts), 1)
|
||||
alert = alerts[0]
|
||||
self.assertEqual(alert.file, "file1.cpp")
|
||||
self.assertEqual(alert.kind, AlertKind.Noexcept)
|
||||
self.assertEqual(alert.line, 520)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
Loading…
Reference in New Issue