Skip to content

Commit 14adf28

Browse files
committed
[clang-tidy] Add a fully custom message to bugprone-unsafe-functions
In some cases, such as when recommending the compiler option _FORTIFY_SOURCE, the current custom message format is clunky. Now, when the reason starts with `>`, the replacement string is omitted., so only the Reason is shown.
1 parent 020b928 commit 14adf28

File tree

4 files changed

+20
-5
lines changed

4 files changed

+20
-5
lines changed

clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,17 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
304304
StringRef Reason =
305305
Entry.Reason.empty() ? "is marked as unsafe" : Entry.Reason.c_str();
306306

307-
if (Entry.Replacement.empty()) {
307+
// Omit the replacement, when a fully-custom reason is given.
308+
if (Reason.consume_front(">")) {
309+
diag(SourceExpr->getExprLoc(), "function %0 %1")
310+
<< FuncDecl << Reason.trim() << SourceExpr->getSourceRange();
311+
// Do not recommend a replacement when it is not present.
312+
} else if (Entry.Replacement.empty()) {
308313
diag(SourceExpr->getExprLoc(),
309314
"function %0 %1; it should not be used")
310315
<< FuncDecl << Reason << Entry.Replacement
311316
<< SourceExpr->getSourceRange();
317+
// Otherwise, emit the replacement.
312318
} else {
313319
diag(SourceExpr->getExprLoc(),
314320
"function %0 %1; '%2' should be used instead")

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,11 @@ Changes in existing checks
270270
an additional matcher that generalizes the copy-and-swap idiom pattern
271271
detection.
272272

273+
- Improved :doc:`bugprone-unsafe-functions
274+
<clang-tidy/checks/bugprone/unsafe-functions>` check by omitting the custom
275+
replacement string, when the Reason starts with the character `>` in the
276+
`CustomFunctions` option.
277+
273278
- Improved :doc:`cppcoreguidelines-prefer-member-initializer
274279
<clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to
275280
avoid false positives on inherited members in class templates.

clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@ The `reason` is optional and is used to provide additional information about the
9898
reasoning behind the replacement. The default reason is `is marked as unsafe`.
9999

100100
If `replacement` is empty, the text `it should not be used` will be shown
101-
instead of the suggestion for a replacement.
101+
instead of the suggestion for a replacement. If the `reason` starts with the
102+
character `>`, the replacement message is disabled, to allow better control over
103+
the suggestions.
102104

103105
As an example, the configuration `^original$, replacement, is deprecated;`
104106
will produce the following diagnostic message.
@@ -114,6 +116,8 @@ qualified name (i.e. ``std::original``), otherwise the regex is matched against
114116
If the regular expression starts with `::` (or `^::`), it is matched against the
115117
fully qualified name (``::std::original``).
116118

119+
The same diagnostic message is produced by the configuration `^original$,,> is deprecated; 'replacement' should be used instead`
120+
117121
.. note::
118122

119123
Fully qualified names can contain template parameters on certain C++ classes, but not on C++ functions.

clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\
2-
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix'}}"
2+
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: \"::name_match,,>is a qualname match, but with a fully 'custom' message;^::prefix_match,,is matched on qualname prefix\"}}"
33
// RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\
44
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match'}}"
55

@@ -11,14 +11,14 @@ void prefix_match_regex();
1111

1212
void f1() {
1313
name_match();
14-
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
14+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match, but with a fully 'custom' message
1515
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead
1616
prefix_match();
1717
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used
1818
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used
1919

2020
name_match_regex();
21-
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match; 'replacement' should be used instead
21+
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match, but with a fully 'custom' message
2222
// no-warning STRICT-REGEX
2323

2424
prefix_match_regex();

0 commit comments

Comments
 (0)