Skip to content

Commit 77e673a

Browse files
AnthonyLatsisAnthonyLatsis
committed
DiagnosticEngine: Print the ID of the wrapped, not wrapper, diagnostic
1 parent 012ac5d commit 77e673a

8 files changed

+216
-82
lines changed

‎include/swift/AST/DiagnosticEngine.h

+11-10
Original file line numberDiff line numberDiff line change
@@ -1530,16 +1530,17 @@ namespace swift {
15301530
public:
15311531
DiagnosticKind declaredDiagnosticKindFor(const DiagID id);
15321532

1533-
/// Get a localized format string for a given `DiagID`. If no localization
1534-
/// available returns the default string for that `DiagID`.
1535-
llvm::StringRef diagnosticStringFor(DiagID id);
1536-
1537-
/// Get a localized format string with an optional diagnostic name appended
1538-
/// to it. The diagnostic name type is defined by
1539-
/// `PrintDiagnosticNamesMode`.
1540-
llvm::StringRef diagnosticStringWithNameFor(
1541-
DiagID id, DiagGroupID groupID,
1542-
PrintDiagnosticNamesMode printDiagnosticNamesMode);
1533+
/// Get a localized format string for the given `DiagID`. If no localization
1534+
/// is available, returns the default string.
1535+
llvm::StringRef getFormatStringForDiagnostic(DiagID id);
1536+
1537+
/// Get a localized format string for the given diagnostic. If no
1538+
/// localization is available, returns the default string.
1539+
///
1540+
/// \param includeDiagnosticName Whether to at all consider embedding the
1541+
/// name of the diagnostic identifier or group, per the setting.
1542+
llvm::StringRef getFormatStringForDiagnostic(const Diagnostic &diagnostic,
1543+
bool includeDiagnosticName);
15431544

15441545
static llvm::StringRef diagnosticIDStringFor(const DiagID id);
15451546

‎lib/AST/DiagnosticEngine.cpp

+28-16
Original file line numberDiff line numberDiff line change
@@ -1429,17 +1429,11 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic,
14291429
}
14301430
}
14311431

1432-
llvm::StringRef format;
1433-
if (includeDiagnosticName) {
1434-
format =
1435-
diagnosticStringWithNameFor(diagnostic.getID(), groupID,
1436-
getPrintDiagnosticNamesMode());
1437-
} else {
1438-
format = diagnosticStringFor(diagnostic.getID());
1439-
}
1432+
auto formatString =
1433+
getFormatStringForDiagnostic(diagnostic, includeDiagnosticName);
14401434

14411435
return DiagnosticInfo(diagnostic.getID(), loc, toDiagnosticKind(behavior),
1442-
format, diagnostic.getArgs(), Category,
1436+
formatString, diagnostic.getArgs(), Category,
14431437
getDefaultDiagnosticLoc(),
14441438
/*child note info*/ {}, diagnostic.getRanges(), fixIts,
14451439
diagnostic.isChildNote());
@@ -1622,18 +1616,24 @@ DiagnosticKind DiagnosticEngine::declaredDiagnosticKindFor(const DiagID id) {
16221616
return storedDiagnosticInfos[(unsigned)id].kind;
16231617
}
16241618

1625-
llvm::StringRef DiagnosticEngine::diagnosticStringFor(DiagID id) {
1619+
llvm::StringRef DiagnosticEngine::getFormatStringForDiagnostic(DiagID id) {
16261620
llvm::StringRef message = diagnosticStrings[(unsigned)id];
16271621
if (auto localizationProducer = localization.get()) {
16281622
message = localizationProducer->getMessageOr(id, message);
16291623
}
16301624
return message;
16311625
}
16321626

1633-
llvm::StringRef DiagnosticEngine::diagnosticStringWithNameFor(
1634-
DiagID id, DiagGroupID groupID,
1635-
PrintDiagnosticNamesMode printDiagnosticNamesMode) {
1636-
auto message = diagnosticStringFor(id);
1627+
llvm::StringRef
1628+
DiagnosticEngine::getFormatStringForDiagnostic(const Diagnostic &diagnostic,
1629+
bool includeDiagnosticName) {
1630+
auto diagID = diagnostic.getID();
1631+
auto message = getFormatStringForDiagnostic(diagID);
1632+
1633+
if (!includeDiagnosticName) {
1634+
return message;
1635+
}
1636+
16371637
auto formatMessageWithName = [&](StringRef message, StringRef name) {
16381638
const int additionalCharsLength = 3; // ' ', '[', ']'
16391639
std::string messageWithName;
@@ -1648,16 +1648,28 @@ llvm::StringRef DiagnosticEngine::diagnosticStringWithNameFor(
16481648
switch (printDiagnosticNamesMode) {
16491649
case PrintDiagnosticNamesMode::None:
16501650
break;
1651-
case PrintDiagnosticNamesMode::Identifier:
1652-
message = formatMessageWithName(message, diagnosticIDStringFor(id));
1651+
case PrintDiagnosticNamesMode::Identifier: {
1652+
// If this diagnostic is a wrapper for another diagnostic, use the ID of
1653+
// the wrapped diagnostic.
1654+
for (const auto &arg : diagnostic.getArgs()) {
1655+
if (arg.getKind() == DiagnosticArgumentKind::Diagnostic) {
1656+
diagID = arg.getAsDiagnostic()->ID;
1657+
break;
1658+
}
1659+
}
1660+
1661+
message = formatMessageWithName(message, diagnosticIDStringFor(diagID));
16531662
break;
1663+
}
16541664
case PrintDiagnosticNamesMode::Group:
1665+
auto groupID = diagnostic.getGroupID();
16551666
if (groupID != DiagGroupID::no_group) {
16561667
message =
16571668
formatMessageWithName(message, getDiagGroupInfoByID(groupID).name);
16581669
}
16591670
break;
16601671
}
1672+
16611673
return message;
16621674
}
16631675

‎lib/IDE/CodeCompletionDiagnostics.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ bool CodeCompletionDiagnostics::getDiagnostics(
7474
typename swift::detail::PassArgument<ArgTypes>::type... VArgs) {
7575
DiagID id = ID.ID;
7676
std::vector<DiagnosticArgument> DiagArgs{std::move(VArgs)...};
77-
auto format = Engine.diagnosticStringFor(id);
77+
auto format = Engine.getFormatStringForDiagnostic(id);
7878
DiagnosticEngine::formatDiagnosticText(Out, format, DiagArgs);
7979
severity = getSeverity(Engine.declaredDiagnosticKindFor(id));
8080

‎lib/PrintAsClang/ModuleContentsWriter.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1086,7 +1086,7 @@ class ModuleWriter {
10861086
// Emit a specific unavailable message when we know why a decl can't be
10871087
// exposed, or a generic message otherwise.
10881088
auto diagString =
1089-
M.getASTContext().Diags.diagnosticStringFor(diag.getID());
1089+
M.getASTContext().Diags.getFormatStringForDiagnostic(diag.getID());
10901090
DiagnosticEngine::formatDiagnosticText(os, diagString, diag.getArgs(),
10911091
DiagnosticFormatOptions());
10921092
os << "\");\n";

‎unittests/AST/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ add_swift_unittest(SwiftASTTests
88
DiagnosticBehaviorTests.cpp
99
DiagnosticConsumerTests.cpp
1010
DiagnosticGroupsTests.cpp
11+
DiagnosticInfoTests.cpp
1112
SourceLocTests.cpp
1213
TestContext.cpp
1314
TypeMatchTests.cpp

‎unittests/AST/DiagnosticBehaviorTests.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ TEST(DiagnosticBehavior, WarnUntilSwiftLangMode) {
6464
[](DiagnosticEngine &diags, const DiagnosticInfo &info) {
6565
EXPECT_EQ(info.Kind, DiagnosticKind::Error);
6666
EXPECT_EQ(info.FormatString,
67-
diags.diagnosticStringFor(
67+
diags.getFormatStringForDiagnostic(
6868
diag::error_immediate_mode_missing_stdlib.ID));
6969
},
7070
/*expectedNumCallbackCalls=*/1);
@@ -77,12 +77,12 @@ TEST(DiagnosticBehavior, WarnUntilSwiftLangMode) {
7777
},
7878
[](DiagnosticEngine &diags, const DiagnosticInfo &info) {
7979
EXPECT_EQ(info.Kind, DiagnosticKind::Warning);
80-
EXPECT_EQ(info.FormatString,
81-
diags.diagnosticStringFor(diag::error_in_swift_lang_mode.ID));
80+
EXPECT_EQ(info.FormatString, diags.getFormatStringForDiagnostic(
81+
diag::error_in_swift_lang_mode.ID));
8282

8383
auto wrappedDiagInfo = info.FormatArgs.front().getAsDiagnostic();
8484
EXPECT_EQ(wrappedDiagInfo->FormatString,
85-
diags.diagnosticStringFor(
85+
diags.getFormatStringForDiagnostic(
8686
diag::error_immediate_mode_missing_stdlib.ID));
8787
},
8888
/*expectedNumCallbackCalls=*/1);
@@ -96,12 +96,12 @@ TEST(DiagnosticBehavior, WarnUntilSwiftLangMode) {
9696
[](DiagnosticEngine &diags, const DiagnosticInfo &info) {
9797
EXPECT_EQ(info.Kind, DiagnosticKind::Warning);
9898
EXPECT_EQ(info.FormatString,
99-
diags.diagnosticStringFor(
99+
diags.getFormatStringForDiagnostic(
100100
diag::error_in_a_future_swift_lang_mode.ID));
101101

102102
auto wrappedDiagInfo = info.FormatArgs.front().getAsDiagnostic();
103103
EXPECT_EQ(wrappedDiagInfo->FormatString,
104-
diags.diagnosticStringFor(
104+
diags.getFormatStringForDiagnostic(
105105
diag::error_immediate_mode_missing_stdlib.ID));
106106
},
107107
/*expectedNumCallbackCalls=*/1);

‎unittests/AST/DiagnosticGroupsTests.cpp

-48
Original file line numberDiff line numberDiff line change
@@ -63,54 +63,6 @@ static void diagnosticGroupsTestCase(
6363
EXPECT_EQ(count, expectedNumCallbackCalls);
6464
}
6565

66-
TEST(DiagnosticGroups, PrintDiagnosticGroups) {
67-
// Test that we append the correct group in the format string.
68-
diagnosticGroupsTestCase(
69-
[](DiagnosticEngine &diags) {
70-
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group);
71-
72-
TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID,
73-
DiagGroupID::DeprecatedDeclaration);
74-
diags.diagnose(SourceLoc(), diagnostic);
75-
},
76-
[](const DiagnosticInfo &info) {
77-
EXPECT_TRUE(info.FormatString.ends_with(" [DeprecatedDeclaration]"));
78-
},
79-
/*expectedNumCallbackCalls=*/1);
80-
81-
diagnosticGroupsTestCase(
82-
[](DiagnosticEngine &diags) {
83-
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group);
84-
85-
TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID,
86-
DiagGroupID::no_group);
87-
diags.diagnose(SourceLoc(), diagnostic);
88-
},
89-
[](const DiagnosticInfo &info) {
90-
EXPECT_FALSE(info.FormatString.ends_with("]"));
91-
},
92-
/*expectedNumCallbackCalls=*/1);
93-
}
94-
95-
TEST(DiagnosticGroups, DiagnosticsWrappersInheritGroups) {
96-
// Test that we don't loose the group of a diagnostic when it gets wrapped in
97-
// another one.
98-
diagnosticGroupsTestCase(
99-
[](DiagnosticEngine &diags) {
100-
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group);
101-
102-
TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID,
103-
DiagGroupID::DeprecatedDeclaration);
104-
diags.diagnose(SourceLoc(), diagnostic)
105-
.limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning, 99);
106-
},
107-
[](const DiagnosticInfo &info) {
108-
EXPECT_EQ(info.ID, diag::error_in_a_future_swift_lang_mode.ID);
109-
EXPECT_TRUE(info.FormatString.ends_with(" [DeprecatedDeclaration]"));
110-
},
111-
/*expectedNumCallbackCalls=*/1);
112-
}
113-
11466
TEST(DiagnosticGroups, TargetAll) {
11567
// Test that uncategorized diagnostics are escalated when escalating all
11668
// warnings.

‎unittests/AST/DiagnosticInfoTests.cpp

+168
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
//===--- DiagnosticInfoTests.cpp --------------------------------*- C++ -*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2025 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "swift/AST/DiagnosticEngine.h"
14+
#include "swift/AST/DiagnosticGroups.h"
15+
#include "swift/AST/DiagnosticsFrontend.h"
16+
#include "swift/Basic/SourceManager.h"
17+
#include "gtest/gtest.h"
18+
19+
using namespace swift;
20+
21+
namespace {
22+
class TestDiagnosticConsumer : public DiagnosticConsumer {
23+
llvm::function_ref<void(const DiagnosticInfo &)> callback;
24+
25+
public:
26+
TestDiagnosticConsumer(decltype(callback) callback) : callback(callback) {}
27+
28+
void handleDiagnostic(SourceManager &SM,
29+
const DiagnosticInfo &Info) override {
30+
this->callback(Info);
31+
}
32+
};
33+
34+
struct TestDiagnostic : public Diagnostic {
35+
TestDiagnostic(DiagID ID, DiagGroupID GroupID) : Diagnostic(ID, GroupID) {}
36+
};
37+
38+
static void
39+
testCase(llvm::function_ref<void(DiagnosticEngine &)> diagnose,
40+
llvm::function_ref<void(DiagnosticEngine &, const DiagnosticInfo &)>
41+
callback,
42+
unsigned expectedNumCallbackCalls) {
43+
SourceManager sourceMgr;
44+
DiagnosticEngine diags(sourceMgr);
45+
46+
unsigned count = 0;
47+
48+
const auto countingCallback = [&](const DiagnosticInfo &info) {
49+
++count;
50+
callback(diags, info);
51+
};
52+
53+
TestDiagnosticConsumer consumer(countingCallback);
54+
diags.addConsumer(consumer);
55+
diagnose(diags);
56+
diags.removeConsumer(consumer);
57+
58+
EXPECT_EQ(count, expectedNumCallbackCalls);
59+
}
60+
61+
// MARK: PrintDiagnosticNamesMode
62+
63+
TEST(DiagnosticInfo, PrintDiagnosticNamesMode_None) {
64+
// Test that we don't embed anything in the format string if told so.
65+
testCase(
66+
[](DiagnosticEngine &diags) {
67+
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::None);
68+
69+
TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID,
70+
DiagGroupID::DeprecatedDeclaration);
71+
diags.diagnose(SourceLoc(), diagnostic);
72+
},
73+
[](DiagnosticEngine &diags, const DiagnosticInfo &info) {
74+
EXPECT_EQ(info.FormatString,
75+
diags.getFormatStringForDiagnostic(
76+
diag::error_immediate_mode_missing_stdlib.ID));
77+
},
78+
/*expectedNumCallbackCalls=*/1);
79+
}
80+
81+
TEST(DiagnosticInfo, PrintDiagnosticNamesMode_Identifier) {
82+
// Test that we embed correct identifier in the format string.
83+
testCase(
84+
[](DiagnosticEngine &diags) {
85+
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Identifier);
86+
87+
diags.diagnose(SourceLoc(), diag::error_immediate_mode_missing_stdlib);
88+
},
89+
[](DiagnosticEngine &diags, const DiagnosticInfo &info) {
90+
EXPECT_TRUE(info.FormatString.ends_with(
91+
" [error_immediate_mode_missing_stdlib]"));
92+
},
93+
/*expectedNumCallbackCalls=*/1);
94+
}
95+
96+
TEST(DiagnosticInfo, PrintDiagnosticNamesMode_Identifier_WrappedDiag) {
97+
// For a wrapper diagnostic, test that we embed the identifier of the wrapped
98+
// diagnostic.
99+
testCase(
100+
[](DiagnosticEngine &diags) {
101+
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Identifier);
102+
103+
diags.diagnose(SourceLoc(), diag::error_immediate_mode_missing_stdlib)
104+
.limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning, 99);
105+
},
106+
[](DiagnosticEngine &diags, const DiagnosticInfo &info) {
107+
EXPECT_EQ(info.ID, diag::error_in_a_future_swift_lang_mode.ID);
108+
EXPECT_TRUE(info.FormatString.ends_with(
109+
" [error_immediate_mode_missing_stdlib]"));
110+
},
111+
/*expectedNumCallbackCalls=*/1);
112+
}
113+
114+
TEST(DiagnosticInfo, PrintDiagnosticNamesMode_Group_NoGroup) {
115+
// Test that we don't embed anything in the format string if the diagnostic
116+
// has no group.
117+
testCase(
118+
[](DiagnosticEngine &diags) {
119+
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group);
120+
121+
TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID,
122+
DiagGroupID::no_group);
123+
diags.diagnose(SourceLoc(), diagnostic);
124+
},
125+
[](DiagnosticEngine &diags, const DiagnosticInfo &info) {
126+
EXPECT_EQ(info.FormatString,
127+
diags.getFormatStringForDiagnostic(
128+
diag::error_immediate_mode_missing_stdlib.ID));
129+
},
130+
/*expectedNumCallbackCalls=*/1);
131+
}
132+
133+
TEST(DiagnosticInfo, PrintDiagnosticNamesMode_Group) {
134+
// Test that we embed the correct group in the format string.
135+
testCase(
136+
[](DiagnosticEngine &diags) {
137+
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group);
138+
139+
TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID,
140+
DiagGroupID::DeprecatedDeclaration);
141+
diags.diagnose(SourceLoc(), diagnostic);
142+
},
143+
[](DiagnosticEngine &, const DiagnosticInfo &info) {
144+
EXPECT_TRUE(info.FormatString.ends_with(" [DeprecatedDeclaration]"));
145+
},
146+
/*expectedNumCallbackCalls=*/1);
147+
}
148+
149+
TEST(DiagnosticInfo, PrintDiagnosticNamesMode_Group_WrappedDiag) {
150+
// For a wrapper diagnostic, test that we embed the group name of the wrapped
151+
// diagnostic.
152+
testCase(
153+
[](DiagnosticEngine &diags) {
154+
diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group);
155+
156+
TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID,
157+
DiagGroupID::DeprecatedDeclaration);
158+
diags.diagnose(SourceLoc(), diagnostic)
159+
.limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning, 99);
160+
},
161+
[](DiagnosticEngine &, const DiagnosticInfo &info) {
162+
EXPECT_EQ(info.ID, diag::error_in_a_future_swift_lang_mode.ID);
163+
EXPECT_TRUE(info.FormatString.ends_with(" [DeprecatedDeclaration]"));
164+
},
165+
/*expectedNumCallbackCalls=*/1);
166+
}
167+
168+
} // end anonymous namespace

0 commit comments

Comments
 (0)