Skip to content

Parse: Offer a fix-it when platform is specified with the wrong case in availability attributes and queries #61234

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
Sep 22, 2022
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
9 changes: 8 additions & 1 deletion include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,10 @@ ERROR(attr_availability_multiple_kinds ,none,
WARNING(attr_availability_invalid_duplicate,none,
"'%0' argument has already been specified", (StringRef))
WARNING(attr_availability_unknown_platform,none,
"unknown platform '%0' for attribute '%1'", (StringRef, StringRef))
"unknown platform '%0' for attribute '%1'", (StringRef, StringRef))
WARNING(attr_availability_suggest_platform,none,
"unknown platform '%0' for attribute '%1'; did you mean '%2'?",
(StringRef, StringRef, StringRef))
ERROR(attr_availability_expected_platform,none,
"expected platform in '%0' attribute", (StringRef))
ERROR(attr_availability_invalid_renamed,none,
Expand Down Expand Up @@ -1897,6 +1900,10 @@ ERROR(avail_query_expected_rparen,PointsToFirstBadToken,

WARNING(avail_query_unrecognized_platform_name,
PointsToFirstBadToken, "unrecognized platform name %0", (Identifier))
WARNING(avail_query_suggest_platform_name,
PointsToFirstBadToken, "unrecognized platform name %0;"
" did you mean '%1'?",
(Identifier, StringRef))

ERROR(avail_query_disallowed_operator, PointsToFirstBadToken,
"'%0' cannot be used in an availability condition", (StringRef))
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/PlatformKind.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ StringRef platformString(PlatformKind platform);
/// or None if such a platform kind does not exist.
Optional<PlatformKind> platformFromString(StringRef Name);

/// Returns a valid platform string if the candidate string would be a valid
/// platform string if its case were adjusted (e.g. "macos" -> "macOS").
Optional<StringRef> caseCorrectedPlatformString(StringRef candidate);

/// Returns a human-readable version of the platform name as a string, suitable
/// for emission in diagnostics (e.g., "macOS").
StringRef prettyPlatformString(PlatformKind platform);
Expand Down
9 changes: 9 additions & 0 deletions lib/AST/PlatformKind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ Optional<PlatformKind> swift::platformFromString(StringRef Name) {
.Default(Optional<PlatformKind>());
}

Optional<StringRef> swift::caseCorrectedPlatformString(StringRef candidate) {
#define AVAILABILITY_PLATFORM(X, PrettyName) \
if (candidate.compare_insensitive(#X) == 0) { \
return StringRef(#X); \
}
#include "swift/AST/PlatformKinds.def"
return None;
}

static bool isApplicationExtensionPlatform(PlatformKind Platform) {
switch (Platform) {
case PlatformKind::macOSApplicationExtension:
Expand Down
21 changes: 17 additions & 4 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ ParserResult<AvailableAttr> Parser::parseExtendedAvailabilitySpecList(
};

StringRef Platform = Tok.getText();
SourceLoc PlatformLoc = Tok.getLoc();

StringRef Message, Renamed;
VersionArg Introduced, Deprecated, Obsoleted;
Expand Down Expand Up @@ -564,8 +565,14 @@ ParserResult<AvailableAttr> Parser::parseExtendedAvailabilitySpecList(
if (AnyArgumentInvalid)
return nullptr;
if (!PlatformKind.hasValue()) {
diagnose(AttrLoc, diag::attr_availability_unknown_platform,
Platform, AttrName);
if (auto CorrectedPlatform = caseCorrectedPlatformString(Platform)) {
diagnose(PlatformLoc, diag::attr_availability_suggest_platform, Platform,
AttrName, *CorrectedPlatform)
.fixItReplace(SourceRange(PlatformLoc), *CorrectedPlatform);
} else {
diagnose(AttrLoc, diag::attr_availability_unknown_platform, Platform,
AttrName);
}
return nullptr;
}

Expand Down Expand Up @@ -1820,8 +1827,14 @@ ParserStatus Parser::parsePlatformVersionInList(StringRef AttrName,
consumeToken();

if (!MaybePlatform.hasValue()) {
diagnose(PlatformLoc, diag::attr_availability_unknown_platform,
platformText, AttrName);
if (auto correctedPlatform = caseCorrectedPlatformString(platformText)) {
diagnose(PlatformLoc, diag::attr_availability_suggest_platform,
platformText, AttrName, *correctedPlatform)
.fixItReplace(SourceRange(PlatformLoc), *correctedPlatform);
} else {
diagnose(PlatformLoc, diag::attr_availability_unknown_platform,
platformText, AttrName);
}
} else if (*MaybePlatform == PlatformKind::none) {
// Wildcards ('*') aren't supported in this kind of list.
diagnose(PlatformLoc, diag::attr_availability_wildcard_ignored,
Expand Down
11 changes: 9 additions & 2 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3952,8 +3952,15 @@ Parser::parsePlatformVersionConstraintSpec() {
platformFromString(PlatformIdentifier.str());

if (!Platform.hasValue() || Platform.getValue() == PlatformKind::none) {
diagnose(Tok, diag::avail_query_unrecognized_platform_name,
PlatformIdentifier);
if (auto CorrectedPlatform =
caseCorrectedPlatformString(PlatformIdentifier.str())) {
diagnose(PlatformLoc, diag::avail_query_suggest_platform_name,
PlatformIdentifier, *CorrectedPlatform)
.fixItReplace(PlatformLoc, *CorrectedPlatform);
} else {
diagnose(PlatformLoc, diag::avail_query_unrecognized_platform_name,
PlatformIdentifier);
}
Platform = PlatformKind::none;
}

Expand Down
3 changes: 3 additions & 0 deletions test/Parse/availability_query.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ if #available(iDishwasherOS 10.51) { // expected-warning {{unrecognized platform
if #available(iDishwasherOS 10.51, *) { // expected-warning {{unrecognized platform name 'iDishwasherOS'}}
}

if #available(macos 10.51, *) { // expected-warning {{unrecognized platform name 'macos'; did you mean 'macOS'?}} {{15-20=macOS}}
}

if #available(OSX 10.51, OSX 10.52, *) { // expected-error {{version for 'macOS' already specified}}
}

Expand Down
3 changes: 3 additions & 0 deletions test/Parse/availability_query_unavailability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ if #unavailable(iDishwasherOS 10.51) { // expected-warning {{unrecognized platfo
if #unavailable(iDishwasherOS 10.51) { // expected-warning {{unrecognized platform name 'iDishwasherOS'}}
}

if #unavailable(macos 10.51) { // expected-warning {{unrecognized platform name 'macos'; did you mean 'macOS'?}} {{17-22=macOS}}
}

if #unavailable(OSX 10.51, OSX 10.52) { // expected-error {{version for 'macOS' already specified}}
}

Expand Down
9 changes: 8 additions & 1 deletion test/Sema/availability_define_parsing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// RUN: -define-availability "_justAName" \
// RUN: -define-availability "_brokenPlatforms:spaceOS 10.11" \
// RUN: -define-availability "_refuseWildcard:iOS 13.0, *, macOS 11.0" \
// RUN: -define-availability "_incorrectCase:ios 13.0, macos 11.0" \
// RUN: -define-availability "_duplicateVersion 1.0:iOS 13.0" \
// RUN: -define-availability "_duplicateVersion 1.0:iOS 13.0" \
// RUN: 2>&1 | %FileCheck %s
Expand All @@ -21,11 +22,17 @@ public func brokenPlatforms() {}
// CHECK: -define-availability argument:1:11: error: expected ':' after '_justAName' in availability macro definition
// CHECK-NEXT: _justAName

// CHECK: -define-availability argument:1:31: warning: unrecognized platform name 'spaceOS'
// CHECK: -define-availability argument:1:18: warning: unrecognized platform name 'spaceOS'
// CHECK-NEXT: _brokenPlatforms:spaceOS 10.11

// CHECK: -define-availability argument:1:27: error: future platforms identified by '*' cannot be used in an availability macro
// CHECK-NEXT: _refuseWildcard

// CHECK: -define-availability argument:1:16: warning: unrecognized platform name 'ios'; did you mean 'iOS'?
// CHECK-NEXT: _incorrectCase

// CHECK: -define-availability argument:1:26: warning: unrecognized platform name 'macos'; did you mean 'macOS'?
// CHECK-NEXT: _incorrectCase

// CHECK: duplicate definition of availability macro '_duplicateVersion' for version '1.0'
// CHECK-NEXT: _duplicateVersion
5 changes: 5 additions & 0 deletions test/Sema/diag_originally_definedin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ public func macroVersioned() {}
// expected-error@-1 {{expected version number in '@_originallyDefinedIn' attribute}}
public func macroUnknown() {}

@available(macOS 10.9, *)
@_originallyDefinedIn(module: "original", macos 10.13) // expected-warning {{unknown platform 'macos' for attribute '@_originallyDefinedIn'; did you mean 'macOS'?}} {{43-48=macOS}}
// expected-error@-1 {{expected at least one platform version in '@_originallyDefinedIn' attribute}}
public func incorrectPlatformCase() {}

@available(macOS 10.9, *)
@_originallyDefinedIn(module: "original", swift 5.1) // expected-warning {{unknown platform 'swift' for attribute '@_originallyDefinedIn'}}
// expected-error@-1 {{expected at least one platform version in '@_originallyDefinedIn' attribute}}
Expand Down
9 changes: 9 additions & 0 deletions test/attr/attr_availability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ func noKind() {}
@available(badPlatform, unavailable) // expected-warning {{unknown platform 'badPlatform' for attribute 'available'}}
func unavailable_bad_platform() {}

@available(macos, unavailable) // expected-warning {{unknown platform 'macos' for attribute 'available'; did you mean 'macOS'?}} {{12-17=macOS}}
func incorrect_platform_case() {}

// Handle unknown platform.
@available(HAL9000, unavailable) // expected-warning {{unknown platform 'HAL9000'}}
func availabilityUnknownPlatform() {}
Expand Down Expand Up @@ -225,6 +228,12 @@ func shortFormWithUnrecognizedPlatform() {
func shortFormWithTwoUnrecognizedPlatforms() {
}

@available(ios 8.0, macos 10.12, *)
// expected-warning@-1 {{unrecognized platform name 'ios'; did you mean 'iOS'?}}
// expected-warning@-2 {{unrecognized platform name 'macos'; did you mean 'macOS'?}}
func shortFormWithTwoPlatformsIncorrectCase() {
}

// Make sure that even after the parser hits an unrecognized
// platform it validates the availability.
@available(iOS 8.0, iDishwasherOS 22.0, iOS 9.0, *)
Expand Down
4 changes: 4 additions & 0 deletions test/attr/attr_backDeploy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ public func transparentFunc() {}

// MARK: - Attribute parsing

@available(macOS 11.0, iOS 14.0, *)
@_backDeploy(before: macos 12.0, iOS 15.0) // expected-warning {{unknown platform 'macos' for attribute '@_backDeploy'; did you mean 'macOS'?}} {{22-27=macOS}}
public func incorrectPlatformCaseFunc() {}

@available(macOS 11.0, *)
@_backDeploy(before: macOS 12.0, unknownOS 1.0) // expected-warning {{unknown platform 'unknownOS' for attribute '@_backDeploy'}}
public func unknownOSFunc() {}
Expand Down
3 changes: 3 additions & 0 deletions test/attr/spi_available.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ public class SPIClass3 {}

@_spi_available(macOS 10.4, *)
public class SPIClass4 {} // expected-warning {{symbols that are @_spi_available on all platforms should use @_spi instead}}

@_spi_available(macos 10.15, *) // expected-warning {{unrecognized platform name 'macos'; did you mean 'macOS'?}} {{17-22=macOS}}
public class SPIClass5 {}