Skip to content

[Sema] Warn about redundant ': Any' conformances #17242

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
wants to merge 2 commits into from
Closed

[Sema] Warn about redundant ': Any' conformances #17242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
101eea1
[Sema] Fix implementation of getStartLocOfInheritanceClause
hamishknight Jun 11, 2018
f4691c0
[Sema] Warn about stated conformances to 'Any'
hamishknight Jun 11, 2018
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
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1724,6 +1724,11 @@ WARNING(redundant_conformance_adhoc_conditional,none,
NOTE(redundant_conformance_witness_ignored,none,
"%0 %1 will not be used to satisfy the conformance to %2",
(DescriptiveDeclKind, DeclName, DeclName))
WARNING(redundant_conformance_warning,none,
"redundant conformance of %0 to %1", (Type, Type))
NOTE(all_types_implicitly_conform_to,none,
"all types implicitly conform to %0", (Type))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative phrasing for the note would be "all types are subtypes of %0", which is more technically correct for Any as it's not actually a protocol you can conform to (just an empty protocol composition) – but I feel the current wording is more user friendly. Thoughts?


// "Near matches"
WARNING(req_near_match,none,
Expand Down
34 changes: 30 additions & 4 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,15 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
resolver = &defaultResolver;

MutableArrayRef<TypeLoc> inheritedClause;
Type declInterfaceTy;

// If we already checked the inheritance clause, don't do so again.
if (auto type = dyn_cast<TypeDecl>(decl)) {
if (type->checkedInheritanceClause())
return;

declInterfaceTy = type->getDeclaredInterfaceType();

// This breaks infinite recursion, which will be diagnosed separately.
type->setCheckedInheritanceClause();
inheritedClause = type->getInherited();
Expand All @@ -343,6 +346,8 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
ext->checkedInheritanceClause())
return;

declInterfaceTy = ext->getExtendedType();

// This breaks infinite recursion, which will be diagnosed separately.
ext->setCheckedInheritanceClause();
inheritedClause = ext->getInherited();
Expand All @@ -363,8 +368,11 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
// Retrieve the location of the start of the inheritance clause.
auto getStartLocOfInheritanceClause = [&] {
if (auto genericTypeDecl = dyn_cast<GenericTypeDecl>(decl)) {
if (auto genericParams = genericTypeDecl->getGenericParams())
return genericParams->getSourceRange().End;
// Get the end location of the generic parameters, except for protocols
// which don't have explicit generic parameters.
if (!isa<ProtocolDecl>(decl))
if (auto genericParams = genericTypeDecl->getGenericParams())
return genericParams->getSourceRange().End;

return genericTypeDecl->getNameLoc();
}
Expand All @@ -373,7 +381,7 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
return typeDecl->getNameLoc();

if (auto ext = dyn_cast<ExtensionDecl>(decl))
return ext->getSourceRange().End;
return ext->getExtendedTypeLoc().getLoc();

return SourceLoc();
};
Expand Down Expand Up @@ -444,8 +452,26 @@ void TypeChecker::checkInheritanceClause(Decl *decl,
if (inheritedTy->hasArchetype() && !isa<GenericTypeParamDecl>(decl))
inheritedTy = inheritedTy->mapTypeOutOfContext();

// Check whether we inherited from the same type twice.
CanType inheritedCanTy = inheritedTy->getCanonicalType();

// Check for a stated conformance to Any, which is redundant.
// Ignore cases of ': Any' *constraints* that are parsed as inheritance
// clauses, such as with protocol and placeholder decls e.g <T : Any>, as
// these should be handled by the generic signature builder along with
// redundant constraints that appear as a part of a 'where' clause.
if (inheritedCanTy == Context.TheAnyType &&
!isa<AbstractTypeParamDecl>(decl) && !isa<ProtocolDecl>(decl)) {
auto diagLoc = inherited.getSourceRange().Start;
auto removalRange = getRemovalRange(i);

diagnose(diagLoc, diag::redundant_conformance_warning, declInterfaceTy,
inheritedTy)
.fixItRemoveChars(removalRange.Start, removalRange.End);
diagnose(diagLoc, diag::all_types_implicitly_conform_to, inheritedTy);
continue;
}

// Check whether we inherited from the same type twice.
auto knownType = inheritedTypes.find(inheritedCanTy);
if (knownType != inheritedTypes.end()) {
// If the duplicated type is 'AnyObject', check whether the first was
Expand Down
47 changes: 47 additions & 0 deletions test/decl/protocol/protocols.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,26 +51,73 @@ protocol FormattedPrintable : CustomStringConvertible {
func print(format: TestFormat)
}

// expected-warning@+2 {{redundant conformance of 'X0' to 'Any'}} {{13-18=}}
// expected-note@+1 {{all types implicitly conform to 'Any'}}
struct X0 : Any, CustomStringConvertible {
func print() {}
}

// expected-warning@+2 {{redundant conformance of 'X1' to 'Any'}} {{12-17=}}
// expected-note@+1 {{all types implicitly conform to 'Any'}}
class X1 : Any, CustomStringConvertible {
func print() {}
}

// expected-warning@+2 {{redundant conformance of 'X2' to 'Any'}} {{8-14=}}
// expected-note@+1 {{all types implicitly conform to 'Any'}}
enum X2 : Any { }

extension X2 : CustomStringConvertible {
func print() {}
}

struct X5 {
func print() {}
}
extension X5 : Any {} // expected-warning {{redundant conformance of 'X5' to 'Any'}} {{13-19=}}
// expected-note@-1 {{all types implicitly conform to 'Any'}}

extension X5 : Any, CustomStringConvertible {} // expected-warning {{redundant conformance of 'X5' to 'Any'}} {{16-21=}}
// expected-note@-1 {{all types implicitly conform to 'Any'}}

struct X6 {
func print() {}
}
extension X6 : CustomStringConvertible, Any {} // expected-warning {{redundant conformance of 'X6' to 'Any'}} {{39-44=}}
// expected-note@-1 {{all types implicitly conform to 'Any'}}

struct X7<T> : Any {} // expected-warning {{redundant conformance of 'X7<T>' to 'Any'}} {{13-19=}}
// expected-note@-1 {{all types implicitly conform to 'Any'}}

struct X8<T> {
func print() {}
}
extension X8 : Any where T : CustomStringConvertible {} // expected-warning {{redundant conformance of 'X8<T>' to 'Any'}} {{13-19=}}
// expected-note@-1 {{all types implicitly conform to 'Any'}}

typealias AnyAlias = Any
class X9 : AnyAlias {} // expected-warning {{redundant conformance of 'X9' to 'AnyAlias' (aka 'Any')}} {{9-20=}}
// expected-note@-1 {{all types implicitly conform to 'AnyAlias' (aka 'Any')}}

struct X10 : Any & Any {} // expected-warning {{redundant conformance of 'X10' to 'Any'}} {{11-23=}}
// expected-note@-1 {{all types implicitly conform to 'Any'}}

// FIX-ME(SR-8009): Handle these cases.
protocol P5 : Any {}
func foo<T : Any>(_ x: T) {}

// Explicit conformance checks (unsuccessful)

// expected-warning@+2 {{redundant conformance of 'NotPrintableS' to 'Any'}} {{24-29=}}
// expected-note@+1 {{all types implicitly conform to 'Any'}}
struct NotPrintableS : Any, CustomStringConvertible {} // expected-error{{type 'NotPrintableS' does not conform to protocol 'CustomStringConvertible'}}

// expected-warning@+2 {{redundant conformance of 'NotPrintableC' to 'Any'}} {{46-51=}}
// expected-note@+1 {{all types implicitly conform to 'Any'}}
class NotPrintableC : CustomStringConvertible, Any {} // expected-error{{type 'NotPrintableC' does not conform to protocol 'CustomStringConvertible'}}

// expected-warning@+2 {{redundant conformance of 'NotPrintableO' to 'Any'}} {{22-27=}}
// expected-note@+1 {{all types implicitly conform to 'Any'}}
enum NotPrintableO : Any, CustomStringConvertible {} // expected-error{{type 'NotPrintableO' does not conform to protocol 'CustomStringConvertible'}}

struct NotFormattedPrintable : FormattedPrintable { // expected-error{{type 'NotFormattedPrintable' does not conform to protocol 'CustomStringConvertible'}}
Expand Down