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 14064 - Error message about @ attributes incomplete #12852

Merged
merged 1 commit into from
Jul 11, 2021

Conversation

dkorpel
Copy link
Contributor

@dkorpel dkorpel commented Jul 9, 2021

Ideally, I would think that the error message would treat the problem a bit like happens with a spelling error for variables, and ask if you meant deprecated instead of @deprecated - at least as long as @deprecated hasn't been declared as a UDA anyway

I've made the @deprecated mistake before as well. For this fix I just check any keyword in case someone accidentally writes @nothrow or @pure. The message doesn't straight up suggest a replacement because e.g. suggesting foreach instead of @foreach in a function signature is nonsensical.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! 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
14064 minor Error message about @ attributes incomplete.

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#12852"

@MoonlightSentinel
Copy link
Contributor

MoonlightSentinel commented Jul 9, 2021

Nice. not an @ attribute sounds redundant, maybe a hint to remove the leading @ would be better (at least for the cases that refer to nothrow, ...)

@dkorpel
Copy link
Contributor Author

dkorpel commented Jul 10, 2021

not an @ attribute sounds redundant

I disagree. I'd consider '@' as just "an @", and pure as just "an attribute", but only @nogc as "an @ attribute".

maybe a hint to remove the leading @ would be better (at least for the cases that refer to nothrow, ...)

I don't want the error message logic to become complex, I rather just treat all keywords the same in the error message.

@MoonlightSentinel
Copy link
Contributor

I disagree. I'd consider '@' as just "an @", and pure as just "an attribute", but only @nogc as "an @ attribute".

Fair enough, the spec also refers to pure and nothrow as attributes.

MoonlightSentinel
MoonlightSentinel approved these changes Jul 10, 2021
@dkorpel dkorpel force-pushed the keyword-attribute-errmsg branch from 8627ade to 425198c Compare July 11, 2021 12:22
@thewilsonator thewilsonator merged commit 5ffb73b into dlang:master Jul 11, 2021
@dkorpel dkorpel deleted the keyword-attribute-errmsg branch July 11, 2021 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants