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

Fix issue 20637: Don't offer private type property corrections #10867

Conversation

FeepingCreature
Copy link
Contributor

Check whether property is visible locally before offering a correction. Pass through scope to getProperty to enable this.

@dlang-bot
Copy link
Contributor

dlang-bot commented Mar 5, 2020

Thanks for your pull request and interest in making D better, @FeepingCreature! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
20637 trivial spelling correction offers private members

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#10867"

@FeepingCreature FeepingCreature changed the title Fix/issue 20637 dont offer private type property corrections Fix issue 20637: Don't offer private type property corrections Mar 5, 2020
@FeepingCreature FeepingCreature force-pushed the fix/issue-20637-dont-offer-private-type-property-corrections branch 2 times, most recently from c25347d to d89b839 Compare March 5, 2020 12:54
@FeepingCreature
Copy link
Contributor Author

Errors seem spurious? Someone seems to have committed a broken state.

Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

This code is really a mess. Half of those properties can't be redefined, the other half can but it doesn't really make sense most of the time (you can redefine offsetof and init).
That search_correct shouldn't really be there, but it just seems to be the last place it gets to for a DotIdExp. Anyway the fix LGTM. For the spurious error, try rebasing on master, the parent of your commit is bea5d22.

@FeepingCreature
Copy link
Contributor Author

Oh! I'd fetched from origin, not upstream, lol.

@FeepingCreature FeepingCreature force-pushed the fix/issue-20637-dont-offer-private-type-property-corrections branch from d89b839 to 388a16e Compare March 5, 2020 13:33
@FeepingCreature
Copy link
Contributor Author

Note that this can be reverted once the issue of private members not printing a proper "private member not accessible" error is fixed.

@Geod24
Copy link
Member

Geod24 commented Mar 5, 2020

Note that this can be reverted once the issue of private members not printing a proper "private member not accessible" error is fixed.

So in this case, why not just go with that fix directly ?
EDIT: I'm happy to go with this diff, but if there's a much more trivial fix that would make this be reverted, why not go with it ? Temporary removing auto-merge to give time for an answer.

@FeepingCreature
Copy link
Contributor Author

Well, primarily because I don't know how to do it. :P

@Geod24
Copy link
Member

Geod24 commented Mar 6, 2020

Do you have a test case for it ? :)

@FeepingCreature
Copy link
Contributor Author

Well, if you look at b7f2e63 , reintroducing something like those would be the way to go. I think they were removed in the context of the deprecation and removal of private access being silently tolerated sometimes.

@Geod24
Copy link
Member

Geod24 commented Mar 6, 2020

Let's go with this. It's simple, it works, and has a test case.

@Geod24 Geod24 merged commit b183809 into dlang:master Mar 6, 2020
@FeepingCreature FeepingCreature deleted the fix/issue-20637-dont-offer-private-type-property-corrections branch March 6, 2020 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants