Merge pull request #40705 from LucianoPAlmeida/SR-15657-diag-enum-cp

[Sema] Avoid misleading diagnostics for incomplete enum computed properties

Author: LucianoPAlmeida

include/swift/AST/DiagnosticsParse.def

@@ -251,8 +251,11 @@ ERROR(enum_case_dot_prefix,none,
 ERROR(static_var_decl_global_scope,none,
       "%select{%error|static properties|class properties}0 may only be declared on a type",
       (StaticSpellingKind))
-ERROR(computed_property_no_accessors, none,
-      "%select{computed property|subscript}0 must have accessors specified", (bool))
+ERROR(unexpected_curly_braces_in_decl, none,
+      "unexpected '{' in declaration", ())
+ERROR(missing_accessor_return_decl,none,
+      "missing return in %select{accessor|subscript}0 expected to return %1",
+      (bool, TypeRepr*))
 ERROR(expected_getset_in_protocol,none,
       "expected get or set in a protocol property", ())
 ERROR(unexpected_getset_implementation_in_protocol,none,

include/swift/Parse/Parser.h

@@ -1169,10 +1169,9 @@ class Parser {
   struct ParsedAccessors;
   ParserStatus parseGetSet(ParseDeclOptions Flags,
                            GenericParamList *GenericParams,
-                           ParameterList *Indices,
+                           ParameterList *Indices, TypeRepr *ResultType,
                            ParsedAccessors &accessors,
-                           AbstractStorageDecl *storage,
-                           SourceLoc StaticLoc);
+                           AbstractStorageDecl *storage, SourceLoc StaticLoc);
   ParserResult<VarDecl> parseDeclVarGetSet(PatternBindingEntry &entry,
                                            ParseDeclOptions Flags,
                                            SourceLoc StaticLoc,

lib/Parse/ParseDecl.cpp

@@ -6226,7 +6226,7 @@ ParserStatus Parser::parseGetEffectSpecifier(ParsedAccessors &accessors,
 
 ParserStatus Parser::parseGetSet(ParseDeclOptions Flags,
                                  GenericParamList *GenericParams,
-                                 ParameterList *Indices,
+                                 ParameterList *Indices, TypeRepr *ResultType,
                                  ParsedAccessors &accessors,
                                  AbstractStorageDecl *storage,
                                  SourceLoc StaticLoc) {
@@ -6258,8 +6258,22 @@ ParserStatus Parser::parseGetSet(ParseDeclOptions Flags,
     if (parsingLimitedSyntax)
       return makeParserSuccess();
 
-    diagnose(accessors.RBLoc, diag::computed_property_no_accessors,
-             /*subscript*/ Indices != nullptr);
+    if (ResultType != nullptr) {
+      // An error type at this point means we couldn't parse
+      // the result type for subscript correctly which will be
+      // already diagnosed as missing result type in declaration.
+      if (ResultType->getKind() == TypeReprKind::Error)
+        return makeParserError();
+
+      diagnose(accessors.RBLoc, diag::missing_accessor_return_decl,
+               /*subscript*/ Indices != nullptr, ResultType);
+    } else {
+      // This is supposed to be a computed property, but we don't
+      // have a result type representation which indicates this is probably not
+      // a well-formed computed property. So we can assume that empty braces
+      // are unexpected at this position for this declaration.
+      diagnose(accessors.LBLoc, diag::unexpected_curly_braces_in_decl);
+    }
     return makeParserError();
   }
 
@@ -6488,9 +6502,11 @@ Parser::parseDeclVarGetSet(PatternBindingEntry &entry, ParseDeclOptions Flags,
 
   // Parse getter and setter.
   ParsedAccessors accessors;
+  auto typedPattern = dyn_cast<TypedPattern>(pattern);
+  auto *resultTypeRepr = typedPattern ? typedPattern->getTypeRepr() : nullptr;
   auto AccessorStatus = parseGetSet(Flags, /*GenericParams=*/nullptr,
-                                    /*Indices=*/nullptr, accessors,
-                                    storage, StaticLoc);
+                                    /*Indices=*/nullptr, resultTypeRepr,
+                                    accessors, storage, StaticLoc);
   if (AccessorStatus.hasCodeCompletion())
     return makeParserCodeCompletionStatus();
   if (AccessorStatus.isErrorOrHasCompletion())
@@ -6500,7 +6516,7 @@ Parser::parseDeclVarGetSet(PatternBindingEntry &entry, ParseDeclOptions Flags,
   if (!PrimaryVar)
     return nullptr;
 
-  if (!isa<TypedPattern>(pattern)) {
+  if (!typedPattern) {
     if (accessors.Get || accessors.Set || accessors.Address ||
         accessors.MutableAddress) {
       SourceLoc locAfterPattern = pattern->getLoc().getAdvancedLoc(
@@ -8029,7 +8045,7 @@ Parser::parseDeclSubscript(SourceLoc StaticLoc,
       Status.setIsParseError();
     }
   } else if (!Status.hasCodeCompletion()) {
-    Status |= parseGetSet(Flags, GenericParams, Indices.get(),
+    Status |= parseGetSet(Flags, GenericParams, Indices.get(), ElementTy.get(),
                           accessors, Subscript, StaticLoc);
   }
 

lib/Sema/TypeCheckStorage.cpp

@@ -3326,6 +3326,7 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
   bool hasModify = storage->getParsedAccessor(AccessorKind::Modify);
   bool hasMutableAddress = storage->getParsedAccessor(AccessorKind::MutableAddress);
 
+  auto *DC = storage->getDeclContext();
   // 'get', 'read', and a non-mutable addressor are all exclusive.
   ReadImplKind readImpl;
   if (storage->getParsedAccessor(AccessorKind::Get)) {
@@ -3354,10 +3355,10 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
       readImpl = ReadImplKind::Stored;
     }
 
-  // Extensions can't have stored properties. If there are braces, assume
-  // this is an incomplete computed property. This avoids an "extensions
-  // must not contain stored properties" error later on.
-  } else if (isa<ExtensionDecl>(storage->getDeclContext()) &&
+  // Extensions and enums can't have stored properties. If there are braces,
+  // assume this is an incomplete computed property. This avoids an
+  // "extensions|enums must not contain stored properties" error later on.
+  } else if ((isa<ExtensionDecl>(DC) || isa<EnumDecl>(DC)) &&
              storage->getBracesRange().isValid()) {
     readImpl = ReadImplKind::Get;
 

test/Constraints/closures.swift

@@ -95,8 +95,8 @@ func r21544303() {
   inSubcall = false
 
   // This is a problem, but isn't clear what was intended.
-  var somethingElse = true {
-  }  // expected-error {{computed property must have accessors specified}}
+  var somethingElse = true { // expected-error {{unexpected '{' in declaration}}
+  }  
   inSubcall = false
 
   var v2 : Bool = false

test/Parse/omit_return.swift

@@ -545,10 +545,10 @@ func ff_implicitMemberAccessEnumCase() -> Unit {
 
 
 var fv_nop: () {
-} // expected-error {{computed property must have accessors specified}}
+} // expected-error {{missing return in accessor expected to return '()'}}
 
 var fv_missing: String {
-} // expected-error {{computed property must have accessors specified}}
+} // expected-error {{missing return in accessor expected to return 'String'}}
 
 var fv_implicit: String {
     "hello"
@@ -1054,12 +1054,12 @@ var fvs_optionalTryImplicit: String? {
 
 enum S_nop {
     subscript() -> () {
-    } // expected-error {{subscript must have accessors specified}}
+    } // expected-error {{missing return in subscript expected to return '()'}}
 }
 
 enum S_missing {
     subscript() -> String {
-    } // expected-error {{subscript must have accessors specified}}
+    } // expected-error {{missing return in subscript expected to return 'String'}}
 }
 
 enum S_implicit {

test/decl/subscript/subscripting.swift

@@ -222,14 +222,20 @@ struct RetOverloadedSubscript {
 
 struct MissingGetterSubscript1 {
   subscript (i : Int) -> Int {
-  } // expected-error {{subscript must have accessors specified}}
+  } // expected-error {{missing return in subscript expected to return 'Int'}}
 }
 struct MissingGetterSubscript2 {
   subscript (i : Int, j : Int) -> Int {
     set {} // expected-error{{subscript with a setter must also have a getter}}
   }
 }
 
+struct MissingReturnTypeAndEmptyBodySubscript {
+  subscript(i: Int) { // expected-error{{expected '->' for subscript element type}}
+  // expected-error@-1{{expected subscripting element type}}
+  }
+}
+
 func test_subscript(_ x2: inout X2, i: Int, j: Int, value: inout Int, no: NoSubscript,
                     ovl: inout OverloadedSubscript, ret: inout RetOverloadedSubscript) {
   no[i] = value // expected-error{{value of type 'NoSubscript' has no subscripts}}

test/decl/var/properties.swift

@@ -378,12 +378,12 @@ var x12: X {
   }
 }
 
-var x13: X {} // expected-error {{computed property must have accessors specified}}
+var x13: X {} // expected-error {{missing return in accessor expected to return 'X'}}
 
 struct X14 {}
 extension X14 {
   var x14: X {
-  } // expected-error {{computed property must have accessors specified}}
+  } // expected-error {{missing return in accessor expected to return 'X'}}
 }
 
 // Type checking problems
@@ -1340,3 +1340,12 @@ class LazyPropInClass {
   lazy var foo: Int = { return 0 } // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}}
   // expected-note@-1 {{Remove '=' to make 'foo' a computed property}}{{21-23=}}{{3-8=}}
 }
+
+// SR-15657
+enum SR15657 {
+  var foo: Int {} // expected-error{{missing return in accessor expected to return 'Int'}}
+}
+
+enum SR15657_G<T> {
+  var foo: T {} // expected-error{{missing return in accessor expected to return 'T'}}
+}

test/decl/var/result_builders.swift

@@ -21,7 +21,7 @@ var global: Int
 // FIXME: should this be allowed?
 @Maker
 var globalWithEmptyImplicitGetter: Int {}
-// expected-error@-1 {{computed property must have accessors specified}}
+// expected-error@-1 {{missing return in accessor expected to return 'Int'}}
 // expected-error@-3 {{result builder attribute 'Maker' can only be applied to a variable if it defines a getter}}
 
 @Maker