From 1a5359bc3b6da023e3f3c428723ea2ed0fc05a92 Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Sun, 8 Oct 2023 18:53:47 +0100 Subject: [PATCH 1/8] implementing required changes --- include/swift/AST/DiagnosticsParse.def | 2 ++ lib/Parse/ParseDecl.cpp | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def index 031eb93099241..f3dbd34317aea 100644 --- a/include/swift/AST/DiagnosticsParse.def +++ b/include/swift/AST/DiagnosticsParse.def @@ -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, diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index ce594a999a4c6..eb21894ddefca 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -8980,6 +8980,18 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags, if (ArgParams.isNull() || ArgParams.hasCodeCompletion()) return ParserStatus(ArgParams); } + + // See if there are generic params. + auto genericParams = maybeParseGenericParams() + .getPtrOrNull(); + if (genericParams) { + auto param = genericParams->getParams().front()->getStartLoc(); + if (param) { + diagnose(param, diag::generic_param_cant_be_used_in_enum_case_decl); + Status.setIsParseError(); + return Status; + } + } // See if there's a raw value expression. SourceLoc EqualsLoc; From 7453af9c384c32747bc666ccd4426ccf5db6c469 Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Sun, 8 Oct 2023 19:25:19 +0100 Subject: [PATCH 2/8] implementing unit tests --- test/decl/enum/enumtest.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/decl/enum/enumtest.swift b/test/decl/enum/enumtest.swift index 9cd20c410568a..8f6f2d09f5b12 100644 --- a/test/decl/enum/enumtest.swift +++ b/test/decl/enum/enumtest.swift @@ -636,3 +636,9 @@ 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) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + case bar(param: T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + case baz // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} +} \ No newline at end of file From 601e11deeaa2788bf35b527136079343b992b9dc Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Mon, 9 Oct 2023 11:38:41 +0100 Subject: [PATCH 3/8] checking if list is not empty and making some improvements --- lib/Parse/ParseDecl.cpp | 19 ++++++++++++------- test/decl/enum/enumtest.swift | 6 ++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index eb21894ddefca..7392a034b7f99 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -8982,14 +8982,19 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags, } // See if there are generic params. - auto genericParams = maybeParseGenericParams() + auto genericResults = maybeParseGenericParams() .getPtrOrNull(); - if (genericParams) { - auto param = genericParams->getParams().front()->getStartLoc(); - if (param) { - diagnose(param, diag::generic_param_cant_be_used_in_enum_case_decl); - Status.setIsParseError(); - return Status; + 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()) { + SmallVector argNamesAfterGenerics; + DefaultArgumentInfo defaultArgsAfterGenerics; + parseSingleParameterClause(ParameterContextKind::EnumElement, + &argNamesAfterGenerics, &defaultArgsAfterGenerics); } } diff --git a/test/decl/enum/enumtest.swift b/test/decl/enum/enumtest.swift index 8f6f2d09f5b12..1062d0ea60a67 100644 --- a/test/decl/enum/enumtest.swift +++ b/test/decl/enum/enumtest.swift @@ -641,4 +641,10 @@ enum EnumCaseWithGenericDeclaration { case foo(T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} case bar(param: T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} case baz // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + case one, two // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + case three, four // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + case five(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, eight } \ No newline at end of file From 109e1388c419fef1e57e4986e12037a345d0d8f8 Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Mon, 9 Oct 2023 14:12:08 +0100 Subject: [PATCH 4/8] improving error handling by allowing types to be checked --- lib/Parse/ParseDecl.cpp | 25 ++++++++----------------- test/decl/enum/enumtest.swift | 3 +++ 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 7392a034b7f99..f2b9392831a9f 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -8969,6 +8969,14 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags, diagnose(CaseLoc, diag::expected_identifier_in_decl, "enum 'case'"); } } + + // 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); + } // See if there's a following argument type. ParserResult ArgParams; @@ -8980,23 +8988,6 @@ 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()) { - SmallVector argNamesAfterGenerics; - DefaultArgumentInfo defaultArgsAfterGenerics; - parseSingleParameterClause(ParameterContextKind::EnumElement, - &argNamesAfterGenerics, &defaultArgsAfterGenerics); - } - } // See if there's a raw value expression. SourceLoc EqualsLoc; diff --git a/test/decl/enum/enumtest.swift b/test/decl/enum/enumtest.swift index 1062d0ea60a67..377b221e337c9 100644 --- a/test/decl/enum/enumtest.swift +++ b/test/decl/enum/enumtest.swift @@ -638,11 +638,14 @@ if case nil = foo2 {} // Okay if case .none?? = foo2 {} // Okay enum EnumCaseWithGenericDeclaration { + // expected-error@+1 {{cannot find type 'T' in scope}} case foo(T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + // expected-error@+1 {{cannot find type 'T' in scope}} case bar(param: T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} case baz // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} case one, two // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} case three, four // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + // expected-error@+1 {{cannot find type 'T' in scope}} case five(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?}} From 67694b7d47d7bcab33f990920e0bf97129751db8 Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Sun, 15 Oct 2023 14:10:41 +0100 Subject: [PATCH 5/8] implementing fixItInsert --- lib/Parse/ParseDecl.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index c63eb2d74adb3..e5b2d65d0b09f 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -8974,8 +8974,16 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags, 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); + + auto genericPargit stamDecl = genericResults->getParams().front(); + + auto enumDecl = dyn_cast(CurDeclContext); + + SourceLoc fixLoc = enumDecl->getNameLoc() + .getAdvancedLoc(enumDecl->getName().getLength()); + + diagnose(genericParamDecl->getStartLoc(), diag::generic_param_cant_be_used_in_enum_case_decl) + .fixItInsert(fixLoc, "<" + genericParamDecl->getNameStr().str() + ">"); } // See if there's a following argument type. From 49026e68fa6b3b4dee97756802b7c800b7257067 Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Sun, 15 Oct 2023 14:12:39 +0100 Subject: [PATCH 6/8] fixing typo --- lib/Parse/ParseDecl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index e5b2d65d0b09f..69647a6b070f9 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -8975,7 +8975,7 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags, .getPtrOrNull(); if (genericResults && !genericResults->getParams().empty()) { - auto genericPargit stamDecl = genericResults->getParams().front(); + auto genericParamDecl = genericResults->getParams().front(); auto enumDecl = dyn_cast(CurDeclContext); From 1be158a34b807a76f31c24c978c27c3c34d80d6b Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Sun, 15 Oct 2023 18:01:15 +0100 Subject: [PATCH 7/8] implementing fixItReplace --- lib/Parse/ParseDecl.cpp | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 69647a6b070f9..cb3dc2ab27f98 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -8973,17 +8973,27 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags, // See if there are generic params. auto genericResults = maybeParseGenericParams() .getPtrOrNull(); - if (genericResults && !genericResults->getParams().empty()) { - + auto enumDecl = dyn_cast(CurDeclContext); + if (genericResults && !genericResults->getParams().empty() && enumDecl) { auto genericParamDecl = genericResults->getParams().front(); - auto enumDecl = dyn_cast(CurDeclContext); - - SourceLoc fixLoc = enumDecl->getNameLoc() - .getAdvancedLoc(enumDecl->getName().getLength()); + std::string fixStr; + llvm::raw_string_ostream OS(fixStr); + genericResults->print(OS); - diagnose(genericParamDecl->getStartLoc(), diag::generic_param_cant_be_used_in_enum_case_decl) - .fixItInsert(fixLoc, "<" + genericParamDecl->getNameStr().str() + ">"); + // Check if enum is already generic. + auto enumGenericParams = enumDecl->getGenericParams(); + if (enumGenericParams && !enumGenericParams->getParams().empty()) { + diagnose(genericParamDecl->getStartLoc(), diag::generic_param_cant_be_used_in_enum_case_decl) + .fixItReplace(enumGenericParams->getSourceRange(), fixStr) + .fixItRemove(genericResults->getSourceRange()); + } else { + SourceLoc insertLoc = enumDecl->getNameLoc() + .getAdvancedLoc(enumDecl->getName().getLength()); + diagnose(genericParamDecl->getStartLoc(), diag::generic_param_cant_be_used_in_enum_case_decl) + .fixItInsert(insertLoc, fixStr) + .fixItRemove(genericResults->getSourceRange()); + } } // See if there's a following argument type. From e2c26c9c63a906c34709a1708a6ec0c63bcb34d2 Mon Sep 17 00:00:00 2001 From: Danilo Camarotto Date: Sun, 15 Oct 2023 18:48:10 +0100 Subject: [PATCH 8/8] updating unit tests --- lib/Parse/ParseDecl.cpp | 8 ++++---- test/decl/enum/enumtest.swift | 27 +++++++++++++++++++-------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index cb3dc2ab27f98..510db5fcb0ffb 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -8985,14 +8985,14 @@ Parser::parseDeclEnumCase(ParseDeclOptions Flags, auto enumGenericParams = enumDecl->getGenericParams(); if (enumGenericParams && !enumGenericParams->getParams().empty()) { diagnose(genericParamDecl->getStartLoc(), diag::generic_param_cant_be_used_in_enum_case_decl) - .fixItReplace(enumGenericParams->getSourceRange(), fixStr) - .fixItRemove(genericResults->getSourceRange()); + .fixItRemove(genericResults->getSourceRange()) + .fixItReplace(enumGenericParams->getSourceRange(), fixStr); } else { SourceLoc insertLoc = enumDecl->getNameLoc() .getAdvancedLoc(enumDecl->getName().getLength()); diagnose(genericParamDecl->getStartLoc(), diag::generic_param_cant_be_used_in_enum_case_decl) - .fixItInsert(insertLoc, fixStr) - .fixItRemove(genericResults->getSourceRange()); + .fixItRemove(genericResults->getSourceRange()) + .fixItInsert(insertLoc, fixStr); } } diff --git a/test/decl/enum/enumtest.swift b/test/decl/enum/enumtest.swift index 377b221e337c9..6dc080d265345 100644 --- a/test/decl/enum/enumtest.swift +++ b/test/decl/enum/enumtest.swift @@ -639,15 +639,26 @@ if case .none?? = foo2 {} // Okay enum EnumCaseWithGenericDeclaration { // expected-error@+1 {{cannot find type 'T' in scope}} - case foo(T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + case foo(T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{11-14=}} {{-2:36-36=}} // expected-error@+1 {{cannot find type 'T' in scope}} - case bar(param: T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} - case baz // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} - case one, two // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} - case three, four // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} + case bar(param: T) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{11-14=}} {{-4:36-36=}} + case baz // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{11-14=}} {{-5:36-36=}} + case one, two // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{16-21=}} {{-6:36-36=}} + case three, four // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{13-22=}} {{-7:36-36=}} // expected-error@+1 {{cannot find type 'T' in scope}} - case five(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 five(param: T), six // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{12-15=}} {{-9:36-36=}} + // expected-error@+2 {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{13-16=}} {{-12:36-36=}} + // expected-error@+1 {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{23-26=}} {{-12:36-36=}} case seven, eight + case nine // OK +} + +enum EnumWithGenericDeclaration { + case foo // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{11-14=}} {{-1:32-41=}} + // expected-error@+1 {{cannot find type 'Key' in scope}} + case bar(param: Key) // expected-error {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{11-16=}} {{-3:32-41=}} + // expected-error@+2 {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{11-14=}} {{-6:32-41=}} + // expected-error@+1 {{generic signature cannot be declared in enum 'case'. did you mean to attach it to enum declaration?}} {{19-22=}} {{-6:32-41=}} + case one, two + case nine // OK } \ No newline at end of file