Skip to content

Improve diagnostics for generics in enum case #69055

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.

Sign up for GitHub

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

Closed
dancamarotto wants to merge 9 commits into from
Closed

Improve diagnostics for generics in enum case #69055

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1a5359b
implementing required changes
dancamarotto Oct 8, 2023
7453af9
implementing unit tests
dancamarotto Oct 8, 2023
601e11d
checking if list is not empty and making some improvements
dancamarotto Oct 9, 2023
109e138
improving error handling by allowing types to be checked
dancamarotto Oct 9, 2023
266fa16
Merge branch 'main' into 69036-generics-enum
dancamarotto Oct 10, 2023
67694b7
implementing fixItInsert
dancamarotto Oct 15, 2023
49026e6
fixing typo
dancamarotto Oct 15, 2023
1be158a
implementing fixItReplace
dancamarotto Oct 15, 2023
e2c26c9
updating unit tests
dancamarotto Oct 15, 2023
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: 2 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ ERROR(number_cant_start_decl_name,none,
(StringRef))
ERROR(expected_identifier_after_case_comma, PointsToFirstBadToken,
"expected identifier after comma in enum 'case' declaration", ())
ERROR(generic_param_cant_be_used_in_enum_case_decl, PointsToFirstBadToken,
"generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?", ())
ERROR(let_cannot_be_computed_property,none,
"'let' declarations cannot be computed properties", ())
ERROR(let_cannot_be_observing_property,none,
Expand Down
17 changes: 17 additions & 0 deletions lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8980,6 +8980,23 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags,
if (ArgParams.isNull() || ArgParams.hasCodeCompletion())
return ParserStatus(ArgParams);
}

// See if there are generic params.
auto genericResults = maybeParseGenericParams()
.getPtrOrNull();
if (genericResults && !genericResults->getParams().empty()) {
auto genericLoc = genericResults->getParams().front()->getStartLoc();
diagnose(genericLoc, diag::generic_param_cant_be_used_in_enum_case_decl);

// check if there's a following argument type after
// generic declaration and parse it to ignore it.
if (Tok.isFollowingLParen()) {
Copy link
Contributor

@mininny mininny Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this diagnosis falls through without returning, you can move this generic result check to be done prior to the previous if statement (which parses the argument type)

Copy link
Contributor Author

@dancamarotto dancamarotto Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good suggestion. done.

SmallVector<Identifier, 4> argNamesAfterGenerics;
DefaultArgumentInfo defaultArgsAfterGenerics;
parseSingleParameterClause(ParameterContextKind::EnumElement,
&argNamesAfterGenerics, &defaultArgsAfterGenerics);
}
}

// See if there's a raw value expression.
SourceLoc EqualsLoc;
Expand Down
12 changes: 12 additions & 0 deletions test/decl/enum/enumtest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,3 +636,15 @@ if case nil = foo1 {} // Okay
if case .none? = foo1 {} // Okay
if case nil = foo2 {} // Okay
if case .none?? = foo2 {} // Okay

enum EnumCaseWithGenericDeclaration {
case foo<T>(T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
Copy link
Contributor

@CodaFi CodaFi Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we offer the second clause here as a fixit that reparents the generic parameters into the parent enum declaration?

Copy link
Contributor Author

@dancamarotto dancamarotto Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably can, I just have no idea how to implement this change however - will take a better look to see if I can come up with something.

Copy link
Contributor Author

@dancamarotto dancamarotto Oct 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

case bar<T>(param: T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
case baz<T> // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
case one, two<T> // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
case three<T>, four // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
case five<T>(param: T), six // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
// expected-error@+2 {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
// expected-error@+1 {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}}
case seven<T>, eight<U>
}