Skip to content

Commit f4c9f99

Browse files
committed
Don't display (fix available) when there's only NOLINT fix
1 parent b606238 commit f4c9f99

File tree

5 files changed

+153
-60
lines changed

5 files changed

+153
-60
lines changed

clang-tools-extra/clangd/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ add_clang_library(clangDaemon STATIC
9898
InlayHints.cpp
9999
JSONTransport.cpp
100100
ModulesBuilder.cpp
101+
NoLintFixes.cpp
101102
PathMapping.cpp
102103
Protocol.cpp
103104
Quality.cpp

clang-tools-extra/clangd/Diagnostics.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "Diagnostics.h"
1010
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
1111
#include "Compiler.h"
12+
#include "NoLintFixes.h"
1213
#include "Protocol.h"
1314
#include "SourceCode.h"
1415
#include "support/Logger.h"
@@ -311,8 +312,18 @@ std::string mainMessage(const Diag &D, const ClangdDiagnosticOptions &Opts) {
311312
std::string Result;
312313
llvm::raw_string_ostream OS(Result);
313314
OS << D.Message;
314-
if (Opts.DisplayFixesCount && !D.Fixes.empty())
315-
OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)"; // TODO
315+
316+
// NOLINT fixes are somewhat not real fixes and to say "(fix available)" when
317+
// the fixes is just to suppress could be misleading.
318+
int RealFixCount = D.Fixes.size();
319+
for (auto const &Fix : D.Fixes) {
320+
if (isClangTidyNoLintFixes(Fix)) {
321+
RealFixCount--;
322+
}
323+
}
324+
325+
if (Opts.DisplayFixesCount && RealFixCount > 0)
326+
OS << " (" << (RealFixCount > 1 ? "fixes" : "fix") << " available)";
316327
// If notes aren't emitted as structured info, add them to the message.
317328
if (!Opts.EmitRelatedLocations)
318329
for (auto &Note : D.Notes) {
@@ -795,8 +806,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
795806
}
796807
if (Message.empty()) // either !SyntheticMessage, or we failed to make one.
797808
Info.FormatDiagnostic(Message);
798-
LastDiag->Fixes.push_back(
799-
Fix{std::string(Message), std::move(Edits), {}});
809+
LastDiag->Fixes.push_back(Fix{std::string(Message), std::move(Edits), {}});
800810
return true;
801811
};
802812

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//===--- NoLintFixes.cpp -------------------------------------------*-
2+
// C++-*-===//
3+
//
4+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
5+
// See https://llvm.org/LICENSE.txt for license information.
6+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "NoLintFixes.h"
11+
#include "../clang-tidy/ClangTidyCheck.h"
12+
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
13+
#include "../clang-tidy/ClangTidyModule.h"
14+
#include "AST.h"
15+
#include "Diagnostics.h"
16+
#include "FeatureModule.h"
17+
#include "SourceCode.h"
18+
#include "clang/AST/ASTContext.h"
19+
#include "clang/Basic/Diagnostic.h"
20+
#include "clang/Basic/DiagnosticIDs.h"
21+
#include "clang/Basic/LLVM.h"
22+
#include "clang/Basic/SourceLocation.h"
23+
#include "clang/Basic/SourceManager.h"
24+
#include "clang/Frontend/CompilerInstance.h"
25+
#include "clang/Frontend/CompilerInvocation.h"
26+
#include "clang/Lex/Lexer.h"
27+
#include "clang/Serialization/ASTWriter.h"
28+
#include "llvm/ADT/STLFunctionalExtras.h"
29+
#include "llvm/ADT/SmallVector.h"
30+
#include "llvm/ADT/StringRef.h"
31+
#include <cassert>
32+
#include <cctype>
33+
#include <optional>
34+
#include <regex>
35+
#include <string>
36+
#include <vector>
37+
38+
namespace clang {
39+
namespace clangd {
40+
41+
std::vector<Fix>
42+
clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
43+
const clang::Diagnostic &Info, const Diag &Diag) {
44+
auto RuleName = CTContext.getCheckName(Diag.ID);
45+
if (
46+
// If this isn't a clang-tidy diag
47+
RuleName.empty() ||
48+
// NOLINT does not work on Serverity Error or above
49+
Diag.Severity >= DiagnosticsEngine::Error ||
50+
// No point adding extra fixes if the Diag is for a different file
51+
!Diag.InsideMainFile) {
52+
return {};
53+
}
54+
55+
auto &SrcMgr = Info.getSourceManager();
56+
auto &DiagLoc = Info.getLocation();
57+
58+
auto F = Fix{};
59+
F.Message = llvm::formatv("ignore [{0}] for this line", RuleName);
60+
auto &E = F.Edits.emplace_back();
61+
62+
auto File = SrcMgr.getFileID(DiagLoc);
63+
auto CodeTilDiag = toSourceCode(
64+
SrcMgr, SourceRange(SrcMgr.getLocForStartOfFile(File), DiagLoc));
65+
66+
auto StartCurLine = CodeTilDiag.find_last_of('\n') + 1;
67+
auto CurLine = CodeTilDiag.substr(StartCurLine);
68+
auto Indent = CurLine.take_while([](char C) { return std::isspace(C); });
69+
70+
auto ExistingNoLintNextLineInsertPos = std::optional<int>();
71+
if (StartCurLine > 0) {
72+
auto StartPrevLine = CodeTilDiag.find_last_of('\n', StartCurLine - 1) + 1;
73+
auto PrevLine =
74+
CodeTilDiag.substr(StartPrevLine, StartCurLine - StartPrevLine - 1);
75+
auto NLPos = PrevLine.find("NOLINTNEXTLINE(");
76+
if (NLPos != StringRef::npos) {
77+
ExistingNoLintNextLineInsertPos =
78+
std::make_optional(PrevLine.find(")", NLPos));
79+
}
80+
}
81+
82+
auto InsertPos = sourceLocToPosition(SrcMgr, DiagLoc);
83+
if (ExistingNoLintNextLineInsertPos) {
84+
E.newText = llvm::formatv(", {0}", RuleName);
85+
InsertPos.line -= 1;
86+
InsertPos.character = *ExistingNoLintNextLineInsertPos;
87+
} else {
88+
E.newText = llvm::formatv("{0}// NOLINTNEXTLINE({1})\n", Indent, RuleName);
89+
InsertPos.character = 0;
90+
}
91+
E.range = {InsertPos, InsertPos};
92+
93+
return {F};
94+
}
95+
96+
const auto MsgRegex = std::regex("ignore \\[.*\\] for this line");
97+
bool isClangTidyNoLintFixes(const Fix &F) {
98+
return std::regex_match(F.Message, MsgRegex);
99+
}
100+
101+
} // namespace clangd
102+
} // namespace clang
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//===--- NoLintFixes.h ------------------------------------------*- C++-*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_NOLINTFIXES_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_NOLINTFIXES_H
11+
12+
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
13+
#include "../clang-tidy/ClangTidyModule.h"
14+
#include "Diagnostics.h"
15+
#include "FeatureModule.h"
16+
#include "clang/Basic/Diagnostic.h"
17+
#include <cassert>
18+
#include <vector>
19+
20+
namespace clang {
21+
namespace clangd {
22+
23+
/// Suggesting to insert "\\ NOLINTNEXTLINE(...)" to suppress clang-tidy
24+
/// diagnostics.
25+
std::vector<Fix>
26+
clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
27+
const clang::Diagnostic &Info, const Diag &Diag);
28+
29+
/// Check if a fix created by clangTidyNoLintFixes().
30+
bool isClangTidyNoLintFixes(const Fix &F);
31+
32+
} // namespace clangd
33+
} // namespace clang
34+
35+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_NOLINTFIXES_H

clang-tools-extra/clangd/ParsedAST.cpp

Lines changed: 1 addition & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "HeuristicResolver.h"
2424
#include "IncludeCleaner.h"
2525
#include "IncludeFixer.h"
26+
#include "NoLintFixes.h"
2627
#include "Preamble.h"
2728
#include "SourceCode.h"
2829
#include "TidyProvider.h"
@@ -67,7 +68,6 @@
6768
#include "llvm/Support/Error.h"
6869
#include "llvm/Support/MemoryBuffer.h"
6970
#include <cassert>
70-
#include <cctype>
7171
#include <cstddef>
7272
#include <iterator>
7373
#include <memory>
@@ -402,61 +402,6 @@ filterFastTidyChecks(const tidy::ClangTidyCheckFactories &All,
402402

403403
} // namespace
404404

405-
std::vector<Fix>
406-
clangTidyNoLintFixes(const clang::tidy::ClangTidyContext &CTContext,
407-
const clang::Diagnostic &Info, const Diag &Diag) {
408-
auto RuleName = CTContext.getCheckName(Diag.ID);
409-
if (
410-
// If this isn't a clang-tidy diag
411-
RuleName.empty() ||
412-
// NOLINT does not work on Serverity Error or above
413-
Diag.Severity >= DiagnosticsEngine::Error ||
414-
// No point adding extra fixes if the Diag is for a different file
415-
!Diag.InsideMainFile) {
416-
return {};
417-
}
418-
419-
auto &SrcMgr = Info.getSourceManager();
420-
auto &DiagLoc = Info.getLocation();
421-
422-
auto F = Fix{};
423-
F.Message = llvm::formatv("ignore [{0}] for this line", RuleName);
424-
auto &E = F.Edits.emplace_back();
425-
426-
auto File = SrcMgr.getFileID(DiagLoc);
427-
auto CodeTilDiag = toSourceCode(
428-
SrcMgr, SourceRange(SrcMgr.getLocForStartOfFile(File), DiagLoc));
429-
430-
auto StartCurLine = CodeTilDiag.find_last_of('\n') + 1;
431-
auto CurLine = CodeTilDiag.substr(StartCurLine);
432-
auto Indent = CurLine.take_while([](char C) { return std::isspace(C); });
433-
434-
auto ExistingNoLintNextLineInsertPos = std::optional<int>();
435-
if (StartCurLine > 0) {
436-
auto StartPrevLine = CodeTilDiag.find_last_of('\n', StartCurLine - 1) + 1;
437-
auto PrevLine =
438-
CodeTilDiag.substr(StartPrevLine, StartCurLine - StartPrevLine - 1);
439-
auto NLPos = PrevLine.find("NOLINTNEXTLINE(");
440-
if (NLPos != StringRef::npos) {
441-
ExistingNoLintNextLineInsertPos =
442-
std::make_optional(PrevLine.find(")", NLPos));
443-
}
444-
}
445-
446-
auto InsertPos = sourceLocToPosition(SrcMgr, DiagLoc);
447-
if (ExistingNoLintNextLineInsertPos) {
448-
E.newText = llvm::formatv(", {0}", RuleName);
449-
InsertPos.line -= 1;
450-
InsertPos.character = *ExistingNoLintNextLineInsertPos;
451-
} else {
452-
E.newText = llvm::formatv("{0}// NOLINTNEXTLINE({1})\n", Indent, RuleName);
453-
InsertPos.character = 0;
454-
}
455-
E.range = {InsertPos, InsertPos};
456-
457-
return {F};
458-
}
459-
460405
std::optional<ParsedAST>
461406
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
462407
std::unique_ptr<clang::CompilerInvocation> CI,

0 commit comments

Comments
 (0)