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

Remove dot character when replacing contextual enum with full name #20915

Closed
wants to merge 1 commit into from

Conversation

maniramezan
Copy link
Contributor

Fixes issue by going back one character when suggesting fixit replacement of contextual enum with fully typed name. All the unit tests passing with this change locally. Couldn't figure out where to add the unit test for this specific change.

Resolves SR-8147.

Fixes https://bugs.swift.org/browse/SR-8147 issue by going back one character when suggesting fixit replacement of contextual enum with fully typed name.
@CodaFi
Copy link
Contributor

CodaFi commented Nov 30, 2018

Hi @maniramezan, thanks for the change. Generally, we don’t change the diagnostic engine when a fixit has the wrong source ranges. Could you instead change the offending replace call to do the right thing and add a test?

@jrose-apple
Copy link
Contributor

This problem's also more subtle than just eating a leading dot: it's perfectly okay to do an unqualified rename of an enum case from "foo" to "bar".

@maniramezan
Copy link
Contributor Author

@jrose-apple Yes, I've been looking into the code to see if I can do something smarter based on the type information but couldn't find a good solution based on my limited knowledge. here is where the error diagnostic is happening and this where the fixit is set. Also, would be nice to be able to change the error message to include . in it.

The eating a leading dot works only because this is the only known fixit message that we show that has dot in it, at least based on the existing tests that all passed locally.

Would appreciate any guidance on making it better. It's only trying to replace after dot with fully qualified name and ignores whether variable full-qualified or not.

@CodaFi
Copy link
Contributor

CodaFi commented Nov 18, 2019

@maniramezan It's been a year and the SR has been released back to the community. If you would like to pursue this change, please send us an updated pull request that responds to the review feedback. We can discuss any implementation details on bugs.swift.org.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants