Skip to content

[Diagnostics][NFC] Add support for using the %select format specifier… #26836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/Diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ Most diagnostics have no reason to change behavior under editor mode. An example

- `%0`, `%1`, etc - Formats the specified diagnostic argument based on its type.

- `%select{a|b|c}0` - Chooses from a list of alternatives, separated by vertical bars, based on the value of the given argument. In this example, a value of 2 in diagnostic argument 0 would result in "c" being output.
- `%select{a|b|c}0` - Chooses from a list of alternatives, separated by vertical bars, based on the value of the given argument. In this example, a value of 2 in diagnostic argument 0 would result in "c" being output. The argument to the %select may be an integer, enum, or StringRef. If it's a StringRef, the specifier acts as an emptiness check.

- `%s0` - Produces an "s" if the given argument is anything other than 1, as meant for an English plural. This isn't particularly localizable without a more general `%plural` form, but most diagnostics try to avoid cases where a plural/singular distinction would be necessary in the first place.

Expand Down
30 changes: 6 additions & 24 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2223,10 +2223,8 @@ ERROR(multiple_override,none,
NOTE(multiple_override_prev,none,
"%0 previously overridden here", (DeclName))

ERROR(override_unavailable,none,
"cannot override %0 which has been marked unavailable", (DeclBaseName))
ERROR(override_unavailable_msg, none,
"cannot override %0 which has been marked unavailable: %1",
ERROR(override_unavailable, none,
"cannot override %0 which has been marked unavailable%select{|: %1}1",
(DeclBaseName, StringRef))

ERROR(override_less_available,none,
Expand Down Expand Up @@ -4118,27 +4116,16 @@ ERROR(dynamic_replacement_replaced_constructor_is_not_convenience, none,

ERROR(availability_decl_unavailable, none,
"%select{getter for |setter for |}0%1 is unavailable"
"%select{ in %3|}2",
(unsigned, DeclName, bool, StringRef))
"%select{ in %3|}2%select{|: %4}4",
(unsigned, DeclName, bool, StringRef, StringRef))

#define REPLACEMENT_DECL_KIND_SELECT "select{| instance method| property}"
ERROR(availability_decl_unavailable_rename, none,
"%select{getter for |setter for |}0%1 has been "
"%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
"'%4'",
(unsigned, DeclName, bool, unsigned, StringRef))

ERROR(availability_decl_unavailable_rename_msg, none,
"%select{getter for |setter for |}0%1 has been "
"%select{renamed to|replaced by}2%" REPLACEMENT_DECL_KIND_SELECT "3 "
"'%4': %5",
"'%4'%select{|: %5}5",
(unsigned, DeclName, bool, unsigned, StringRef, StringRef))

ERROR(availability_decl_unavailable_msg, none,
"%select{getter for |setter for |}0%1 is unavailable"
"%select{ in %3|}2: %4",
(unsigned, DeclName, bool, StringRef, StringRef))

NOTE(availability_marked_unavailable, none,
"%select{getter for |setter for |}0%1 has been explicitly marked "
"unavailable here", (unsigned, DeclName))
Expand All @@ -4153,12 +4140,7 @@ NOTE(availability_obsoleted, none,

WARNING(availability_deprecated, none,
"%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
"deprecated%select{| in %3%select{| %5}4}2",
(unsigned, DeclName, bool, StringRef, bool, llvm::VersionTuple))

WARNING(availability_deprecated_msg, none,
"%select{getter for |setter for |}0%1 %select{is|%select{is|was}4}2 "
"deprecated%select{| in %3%select{| %5}4}2: %6",
"deprecated%select{| in %3%select{| %5}4}2%select{|: %6}6",
(unsigned, DeclName, bool, StringRef, bool, llvm::VersionTuple,
StringRef))

Expand Down
10 changes: 8 additions & 2 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,14 @@ static void formatDiagnosticArgument(StringRef Modifier,
break;

case DiagnosticArgumentKind::String:
assert(Modifier.empty() && "Improper modifier for string argument");
Out << Arg.getAsString();
if (Modifier == "select") {
formatSelectionArgument(ModifierArguments, Args,
Arg.getAsString().empty() ? 0 : 1, FormatOpts,
Out);
} else {
assert(Modifier.empty() && "Improper modifier for string argument");
Out << Arg.getAsString();
}
break;

case DiagnosticArgumentKind::Identifier:
Expand Down
54 changes: 19 additions & 35 deletions lib/Sema/TypeCheckAvailability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1955,8 +1955,9 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
if (Attr->Message.empty() && Attr->Rename.empty()) {
diagnose(ReferenceRange.Start, diag::availability_deprecated,
RawAccessorKind, Name, Attr->hasPlatform(), Platform,
Attr->Deprecated.hasValue(), DeprecatedVersion)
.highlight(Attr->getRange());
Attr->Deprecated.hasValue(), DeprecatedVersion,
/*message*/ StringRef())
.highlight(Attr->getRange());
return;
}

Expand All @@ -1967,11 +1968,11 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,

if (!Attr->Message.empty()) {
EncodedDiagnosticMessage EncodedMessage(Attr->Message);
diagnose(ReferenceRange.Start, diag::availability_deprecated_msg,
diagnose(ReferenceRange.Start, diag::availability_deprecated,
RawAccessorKind, Name, Attr->hasPlatform(), Platform,
Attr->Deprecated.hasValue(), DeprecatedVersion,
EncodedMessage.Message)
.highlight(Attr->getRange());
.highlight(Attr->getRange());
} else {
unsigned rawReplaceKind = static_cast<unsigned>(
replacementDeclKind.getValueOr(ReplacementDeclKind::None));
Expand All @@ -1998,12 +1999,9 @@ void swift::diagnoseUnavailableOverride(ValueDecl *override,
ASTContext &ctx = override->getASTContext();
auto &diags = ctx.Diags;
if (attr->Rename.empty()) {
if (attr->Message.empty())
diags.diagnose(override, diag::override_unavailable,
override->getBaseName());
else
diags.diagnose(override, diag::override_unavailable_msg,
override->getBaseName(), attr->Message);
EncodedDiagnosticMessage EncodedMessage(attr->Message);
diags.diagnose(override, diag::override_unavailable,
override->getBaseName(), EncodedMessage.Message);

DeclName name;
unsigned rawAccessorKind;
Expand Down Expand Up @@ -2185,22 +2183,12 @@ bool swift::diagnoseExplicitUnavailability(
unsigned rawReplaceKind = static_cast<unsigned>(
replaceKind.getValueOr(ReplacementDeclKind::None));
StringRef newName = replaceKind ? newNameBuf.str() : Attr->Rename;

if (Attr->Message.empty()) {
auto diag = diags.diagnose(Loc,
diag::availability_decl_unavailable_rename,
RawAccessorKind, Name,
replaceKind.hasValue(),
rawReplaceKind, newName);
attachRenameFixIts(diag);
} else {
EncodedDiagnosticMessage EncodedMessage(Attr->Message);
auto diag =
diags.diagnose(Loc, diag::availability_decl_unavailable_rename_msg,
RawAccessorKind, Name, replaceKind.hasValue(),
rawReplaceKind, newName, EncodedMessage.Message);
diags.diagnose(Loc, diag::availability_decl_unavailable_rename,
RawAccessorKind, Name, replaceKind.hasValue(),
rawReplaceKind, newName, EncodedMessage.Message);
attachRenameFixIts(diag);
}
} else if (isSubscriptReturningString(D, ctx)) {
diags.diagnose(Loc, diag::availabilty_string_subscript_migration)
.highlight(R)
Expand All @@ -2209,16 +2197,12 @@ bool swift::diagnoseExplicitUnavailability(

// Skip the note emitted below.
return true;
} else if (Attr->Message.empty()) {
diags.diagnose(Loc, diag::availability_decl_unavailable,
RawAccessorKind, Name, platform.empty(), platform)
.highlight(R);
} else {
EncodedDiagnosticMessage EncodedMessage(Attr->Message);
diags.diagnose(Loc, diag::availability_decl_unavailable_msg,
RawAccessorKind, Name, platform.empty(), platform,
EncodedMessage.Message)
.highlight(R);
diags
.diagnose(Loc, diag::availability_decl_unavailable, RawAccessorKind,
Name, platform.empty(), platform, EncodedMessage.Message)
.highlight(R);
}

switch (Attr->getVersionAvailability(ctx)) {
Expand Down Expand Up @@ -2695,7 +2679,7 @@ AvailabilityWalker::diagnoseIncDecRemoval(const ValueDecl *D, SourceRange R,
std::tie(RawAccessorKind, Name) = getAccessorKindAndNameForDiagnostics(D);

// If we emit a deprecation diagnostic, produce a fixit hint as well.
auto diag = TC.diagnose(R.Start, diag::availability_decl_unavailable_msg,
auto diag = TC.diagnose(R.Start, diag::availability_decl_unavailable,
RawAccessorKind, Name, true, "",
"it has been removed in Swift 3");
if (isa<PrefixUnaryExpr>(call)) {
Expand Down Expand Up @@ -2746,9 +2730,9 @@ AvailabilityWalker::diagnoseMemoryLayoutMigration(const ValueDecl *D,
std::tie(RawAccessorKind, Name) = getAccessorKindAndNameForDiagnostics(D);

EncodedDiagnosticMessage EncodedMessage(Attr->Message);
auto diag = TC.diagnose(R.Start, diag::availability_decl_unavailable_msg,
RawAccessorKind, Name, true, "",
EncodedMessage.Message);
auto diag =
TC.diagnose(R.Start, diag::availability_decl_unavailable, RawAccessorKind,
Name, true, "", EncodedMessage.Message);
diag.highlight(R);

auto subject = args->getSubExpr();
Expand Down
10 changes: 5 additions & 5 deletions lib/Sema/TypeCheckPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1414,11 +1414,11 @@ bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
EEP->getName().str() == "Some") {
SmallString<4> Rename;
camel_case::toLowercaseWord(EEP->getName().str(), Rename);
diagnose(EEP->getLoc(),
diag::availability_decl_unavailable_rename,
/*"getter" prefix*/2, EEP->getName(), /*replaced*/false,
/*special kind*/0, Rename.str())
.fixItReplace(EEP->getLoc(), Rename.str());
diagnose(
EEP->getLoc(), diag::availability_decl_unavailable_rename,
/*"getter" prefix*/ 2, EEP->getName(), /*replaced*/ false,
/*special kind*/ 0, Rename.str(), /*message*/ StringRef())
.fixItReplace(EEP->getLoc(), Rename.str());

return true;
}
Expand Down