Skip to content
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.

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

[SR-13388] Add Fix-Its to "override" mismatch #55828

Open
DougGregor opened this issue Aug 13, 2020 · 9 comments
Open

[SR-13388] Add Fix-Its to "override" mismatch #55828

DougGregor opened this issue Aug 13, 2020 · 9 comments
Labels
compiler The Swift compiler itself good first issue Good for newcomers improvement

Comments

@DougGregor
Copy link
Member

Previous ID SR-13388
Radar rdar://problem/66539331
Original Reporter @DougGregor
Type Improvement
Status In Progress
Resolution
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Improvement, StarterBug
Assignee RoyCao (JIRA)
Priority Medium

md5: be871146c13ac26dbbf04275c83c2233

Issue Description:

For code like this following:

class Base {
  func foo(a: Int, b: Int) {}
}
class C: Base {
  override func foo(a: Int) {} // error: method does not override any method from its superclass
}

we should provide a Fix-It that adds ", b: Int" to C.foo.

@typesanitizer
Copy link

@swift-ci create

@swift-ci
Copy link
Contributor

Comment by CaoMing (JIRA)

I am interested in implementing this🙂

@typesanitizer
Copy link

Sure thing. Check out our README for instructions on how to understand the workflow of submitting a PR and building the compiler.

In this particular case, you can check which parts of the compiler emit the error by searching for the error string and working backwards from there (error string -> error name -> places which emit the error).

Once you find that, you'll need to see which potential methods could've been over-ridden (but aren't because of the type mismatch). There may be zero or more "matches". The zero match case will not require any fix-its. The one match case will have one fix-it (search for fixit and fixitReplace in the compiler to see examples). When there is more than one match, there is a design decision to be made:

1. If there are multiple matches, show multiple fix-its. This may get overwhelming if there are too many matches.
2. If there are multiple matches, just list the potential matches somehow but no fix-it.
3. If there are multiple matches, don't do anything extra.

@swift-ci
Copy link
Contributor

Comment by CaoMing (JIRA)

Very detailed guidance, thank you very much

@slavapestov
Copy link
Contributor

> Once you find that, you'll need to see which potential methods could've been over-ridden (but aren't because of the type mismatch).

This should already be available in some form. Override matching does a name lookup to find candidates, and then filters this list by argument label and type. You can preserve the original list, and use it to emit diagnostics if the final set of candidates is empty.

@swift-ci
Copy link
Contributor

Comment by CaoMing (JIRA)

class Base {
func foo(a: Int, b: Int, c) {}
}

class C: Base {
override func foo(a: Int) {} // error: method does not override any method from its superclass

}

In this case, we should provide a Fix-It that adds ", b: Int, c: Int" to C.foo, right?

class Base {
func foo(a: Int, b: Int, c: Int) {}
}

class C: Base {
override func foo(a: Int, b: Int) {} // error: method does not override any method from its superclass

}

In this case, we should provide a Fix-It that adds ", c: Int" to C.foo, right?

class Base {
func foo(a: Int, b: Int, c) {}
func foo(a: Int, b: Int, c: Int) {}
}

class C: Base {
override func foo(a: Int) {} // error: method does not override any method from its superclass

}

In this case, we should provide a Fix-It that adds ", b: Int" to C.foo, right?

@theblixguy
Copy link
Collaborator

Yeah, that makes sense to me, there will be one fix-it for each “near-match”.

@swift-ci
Copy link
Contributor

Comment by CaoMing (JIRA)

class Base {

func foo(a: Int, b: Int) {}

}

class C: Base {

override func foo() {} // error: method does not override any method from its superclass

}

In this case, do we provide a Fix-It that adds "a: Int, b: Int" to C.foo?

theindigamer (JIRA User) @theblixguy

@swift-ci
Copy link
Contributor

Comment by CaoMing (JIRA)

@slavapestov @theblixguy Could you help to review and give me some suggestion? Thank you.

#34003

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler The Swift compiler itself good first issue Good for newcomers improvement
Projects
None yet
Development

No branches or pull requests

5 participants