Skip to content

Commit cc49c61

Browse files
theblixguytheblixguy
authored
[Diagnostics] Update diagnostic message for invalid overrides (swiftlang#33097)
* [Diagnostics] Update 'does not override' diagnostic message to include protocol context as well * [Sema] Check whether the override context is a class or a protocol for diagnostic purposes * [Test] Update tests with new diagnostic message for overrides in protocol context * [Sema] Adjust diagnostic for overrides in structs and enums to use the existing 'override_nonclass_decl' diagnostic
1 parent 0c7894f commit cc49c61

File tree

7 files changed

+76
-30
lines changed

7 files changed

+76
-30
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2442,14 +2442,14 @@ ERROR(override_of_non_open,none,
24422442
(DescriptiveDeclKind))
24432443

24442444
ERROR(method_does_not_override,none,
2445-
"method does not override any method from its superclass", ())
2445+
"method does not override any method from its %select{parent protocol|superclass}0", (bool))
24462446
ERROR(property_does_not_override,none,
2447-
"property does not override any property from its superclass", ())
2447+
"property does not override any property from its %select{parent protocol|superclass}0", (bool))
24482448
ERROR(subscript_does_not_override,none,
2449-
"subscript does not override any subscript from its superclass", ())
2449+
"subscript does not override any subscript from its %select{parent protocol|superclass}0", (bool))
24502450
ERROR(initializer_does_not_override,none,
24512451
"initializer does not override a designated initializer from its "
2452-
"superclass", ())
2452+
"%select{parent protocol|superclass}0", (bool))
24532453
ERROR(failable_initializer_override,none,
24542454
"failable initializer %0 cannot override a non-failable initializer",
24552455
(DeclName))

‎lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -525,16 +525,18 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
525525
break;
526526
case OverrideCheckingAttempt::MismatchedOptional:
527527
case OverrideCheckingAttempt::MismatchedTypes:
528-
case OverrideCheckingAttempt::BaseNameWithMismatchedOptional:
528+
case OverrideCheckingAttempt::BaseNameWithMismatchedOptional: {
529+
auto isClassContext = decl->getDeclContext()->getSelfClassDecl() != nullptr;
530+
auto diag = diag::method_does_not_override;
529531
if (isa<ConstructorDecl>(decl))
530-
diags.diagnose(decl, diag::initializer_does_not_override);
532+
diag = diag::initializer_does_not_override;
531533
else if (isa<SubscriptDecl>(decl))
532-
diags.diagnose(decl, diag::subscript_does_not_override);
534+
diag = diag::subscript_does_not_override;
533535
else if (isa<VarDecl>(decl))
534-
diags.diagnose(decl, diag::property_does_not_override);
535-
else
536-
diags.diagnose(decl, diag::method_does_not_override);
536+
diag = diag::property_does_not_override;
537+
diags.diagnose(decl, diag, isClassContext);
537538
break;
539+
}
538540
case OverrideCheckingAttempt::Final:
539541
llvm_unreachable("should have exited already");
540542
}
@@ -1146,6 +1148,7 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
11461148
// If this is an exact type match, we're successful!
11471149
Type declTy = getDeclComparisonType();
11481150
Type owningTy = dc->getDeclaredInterfaceType();
1151+
auto isClassContext = classDecl != nullptr;
11491152
if (declIUOAttr == matchDeclIUOAttr && declTy->isEqual(baseTy)) {
11501153
// Nothing to do.
11511154

@@ -1154,7 +1157,7 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
11541157
auto diagKind = diag::method_does_not_override;
11551158
if (isa<ConstructorDecl>(method))
11561159
diagKind = diag::initializer_does_not_override;
1157-
diags.diagnose(decl, diagKind);
1160+
diags.diagnose(decl, diagKind, isClassContext);
11581161
noteFixableMismatchedTypes(decl, baseDecl);
11591162
diags.diagnose(baseDecl, diag::overridden_near_match_here,
11601163
baseDecl->getDescriptiveKind(),
@@ -1186,7 +1189,7 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
11861189
}
11871190

11881191
if (attempt == OverrideCheckingAttempt::MismatchedTypes) {
1189-
diags.diagnose(decl, diag::subscript_does_not_override);
1192+
diags.diagnose(decl, diag::subscript_does_not_override, isClassContext);
11901193
noteFixableMismatchedTypes(decl, baseDecl);
11911194
diags.diagnose(baseDecl, diag::overridden_near_match_here,
11921195
baseDecl->getDescriptiveKind(),

‎lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,8 +1452,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
14521452
auto overridden = VD->getOverriddenDecl();
14531453
if (auto *OA = VD->getAttrs().getAttribute<OverrideAttr>()) {
14541454
if (!overridden) {
1455-
VD->diagnose(diag::property_does_not_override)
1456-
.highlight(OA->getLocation());
1455+
auto DC = VD->getDeclContext();
1456+
auto isClassContext = DC->getSelfClassDecl() != nullptr;
1457+
auto isStructOrEnumContext = DC->getSelfEnumDecl() != nullptr ||
1458+
DC->getSelfStructDecl() != nullptr;
1459+
if (isStructOrEnumContext) {
1460+
VD->diagnose(diag::override_nonclass_decl)
1461+
.highlight(OA->getLocation())
1462+
.fixItRemove(OA->getRange());
1463+
} else {
1464+
VD->diagnose(diag::property_does_not_override, isClassContext)
1465+
.highlight(OA->getLocation());
1466+
}
14571467
OA->setInvalid();
14581468
}
14591469
}
@@ -1659,8 +1669,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
16591669
// anything, complain.
16601670
if (auto *OA = SD->getAttrs().getAttribute<OverrideAttr>()) {
16611671
if (!SD->getOverriddenDecl()) {
1662-
SD->diagnose(diag::subscript_does_not_override)
1663-
.highlight(OA->getLocation());
1672+
auto DC = SD->getDeclContext();
1673+
auto isClassContext = DC->getSelfClassDecl() != nullptr;
1674+
auto isStructOrEnumContext = DC->getSelfEnumDecl() != nullptr ||
1675+
DC->getSelfStructDecl() != nullptr;
1676+
if (isStructOrEnumContext) {
1677+
SD->diagnose(diag::override_nonclass_decl)
1678+
.highlight(OA->getLocation())
1679+
.fixItRemove(OA->getRange());
1680+
} else {
1681+
SD->diagnose(diag::subscript_does_not_override, isClassContext)
1682+
.highlight(OA->getLocation());
1683+
}
16641684
OA->setInvalid();
16651685
}
16661686
}
@@ -2236,8 +2256,18 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
22362256
// override anything, complain.
22372257
if (auto *OA = FD->getAttrs().getAttribute<OverrideAttr>()) {
22382258
if (!FD->getOverriddenDecl()) {
2239-
FD->diagnose(diag::method_does_not_override)
2240-
.highlight(OA->getLocation());
2259+
auto DC = FD->getDeclContext();
2260+
auto isClassContext = DC->getSelfClassDecl() != nullptr;
2261+
auto isStructOrEnumContext = DC->getSelfEnumDecl() != nullptr ||
2262+
DC->getSelfStructDecl() != nullptr;
2263+
if (isStructOrEnumContext) {
2264+
FD->diagnose(diag::override_nonclass_decl)
2265+
.highlight(OA->getLocation())
2266+
.fixItRemove(OA->getRange());
2267+
} else {
2268+
FD->diagnose(diag::method_does_not_override, isClassContext)
2269+
.highlight(OA->getLocation());
2270+
}
22412271
OA->setInvalid();
22422272
}
22432273
}
@@ -2482,14 +2512,24 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
24822512
// Check whether this initializer overrides an initializer in its
24832513
// superclass.
24842514
if (!checkOverrides(CD)) {
2515+
auto DC = CD->getDeclContext();
2516+
auto isClassContext = DC->getSelfClassDecl() != nullptr;
2517+
auto isStructOrEnumContext = DC->getSelfEnumDecl() != nullptr ||
2518+
DC->getSelfStructDecl() != nullptr;
24852519
// If an initializer has an override attribute but does not override
24862520
// anything or overrides something that doesn't need an 'override'
24872521
// keyword (e.g., a convenience initializer), complain.
24882522
// anything, or overrides something that complain.
24892523
if (auto *attr = CD->getAttrs().getAttribute<OverrideAttr>()) {
24902524
if (!CD->getOverriddenDecl()) {
2491-
CD->diagnose(diag::initializer_does_not_override)
2492-
.highlight(attr->getLocation());
2525+
if (isStructOrEnumContext) {
2526+
CD->diagnose(diag::override_nonclass_decl)
2527+
.highlight(attr->getLocation())
2528+
.fixItRemove(attr->getRange());
2529+
} else {
2530+
CD->diagnose(diag::initializer_does_not_override, isClassContext)
2531+
.highlight(attr->getLocation());
2532+
}
24932533
attr->setInvalid();
24942534
} else if (attr->isImplicit()) {
24952535
// Don't diagnose implicit attributes.
@@ -2513,7 +2553,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
25132553
reqInit->diagnose(diag::overridden_required_initializer_here);
25142554
} else {
25152555
// We tried to override a convenience initializer.
2516-
CD->diagnose(diag::initializer_does_not_override)
2556+
CD->diagnose(diag::initializer_does_not_override, isClassContext)
25172557
.highlight(attr->getLocation());
25182558
CD->getOverriddenDecl()->diagnose(
25192559
diag::convenience_init_override_here);

‎localization/diagnostics/en.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6108,20 +6108,20 @@
61086108
61096109
- id: method_does_not_override
61106110
msg: >-
6111-
method does not override any method from its superclass
6111+
method does not override any method from its %select{parent protocol|superclass}0
61126112
61136113
- id: property_does_not_override
61146114
msg: >-
6115-
property does not override any property from its superclass
6115+
property does not override any property from its %select{parent protocol|superclass}0
61166116
61176117
- id: subscript_does_not_override
61186118
msg: >-
6119-
subscript does not override any subscript from its superclass
6119+
subscript does not override any subscript from its %select{parent protocol|superclass}0
61206120
61216121
- id: initializer_does_not_override
61226122
msg: >-
61236123
initializer does not override a designated initializer from its
6124-
superclass
6124+
%select{parent protocol|superclass}0
61256125
61266126
- id: failable_initializer_override
61276127
msg: >-

‎test/Compatibility/attr_override.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,15 @@ struct S {
147147
override func f() { } // expected-error{{'override' can only be specified on class members}} {{3-12=}}
148148
}
149149
extension S {
150-
override func ef() {} // expected-error{{method does not override any method from its superclass}}
150+
override func ef() {} // expected-error{{'override' can only be specified on class members}} {{3-12=}}
151151
}
152152

153153
enum E {
154154
override func f() { } // expected-error{{'override' can only be specified on class members}} {{3-12=}}
155155
}
156156

157157
protocol P {
158-
override func f() // FIXME wording: expected-error{{method does not override any method from its superclass}}
158+
override func f() // expected-error{{method does not override any method from its parent protocol}}
159159
}
160160

161161
override func f() { } // expected-error{{'override' can only be specified on class members}} {{1-10=}}

‎test/attr/attr_override.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,15 +211,18 @@ struct S {
211211
override func f() { } // expected-error{{'override' can only be specified on class members}} {{3-12=}}
212212
}
213213
extension S {
214-
override func ef() {} // expected-error{{method does not override any method from its superclass}}
214+
override func ef() {} // expected-error{{'override' can only be specified on class members}} {{3-12=}}
215215
}
216216

217217
enum E {
218218
override func f() { } // expected-error{{'override' can only be specified on class members}} {{3-12=}}
219219
}
220220

221221
protocol P {
222-
override func f() // FIXME wording: expected-error{{method does not override any method from its superclass}}
222+
override func f() // expected-error{{method does not override any method from its parent protocol}}
223+
override var g: Int { get } // expected-error{{property does not override any property from its parent protocol}}
224+
override subscript(h: Int) -> Bool { get } // expected-error{{subscript does not override any subscript from its parent protocol}}
225+
override init(i: Int) // expected-error{{initializer does not override a designated initializer from its parent protocol}}
223226
}
224227

225228
override func f() { } // expected-error{{'override' can only be specified on class members}} {{1-10=}}

‎test/decl/protocol/override.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ protocol P6: P0 {
9797

9898
// Complain if 'override' is present but there is no overridden declaration.
9999
protocol P7: P0 {
100-
// expected-error@+1{{method does not override any method from its superclass}}
100+
// expected-error@+1{{method does not override any method from its parent protocol}}
101101
override func foo() -> Int
102102

103103
// expected-error@+1{{property 'prop' with type 'Int' cannot override a property with type 'Self.A'}}

0 commit comments

Comments
 (0)