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

Tracking issue for lints for the html! macro. #1334

Open
2 of 3 tasks
teymour-aldridge opened this issue Jun 20, 2020 · 6 comments
Open
2 of 3 tasks

Tracking issue for lints for the html! macro. #1334

teymour-aldridge opened this issue Jun 20, 2020 · 6 comments
Labels
A-yew-macro Area: The yew-macro crate feature-accepted A feature request that has been accepted and needs implementing feature-request A feature request

Comments

@teymour-aldridge
Copy link
Contributor

teymour-aldridge commented Jun 20, 2020

Describe the feature you'd like
The html! macro should output warnings. The developer should be able to ignore these warnings as they see fit.

These warnings should integrate into the compiler and work just as any other lint/warning would.

Is your feature request related to a problem? Please describe. (Optional)
Often it's easy to make mistakes based on

Describe alternatives you've considered (Optional)
A clear and concise description of any alternative solutions or features you've considered.

Additional context (Optional)
This is blocked on the diagnostics API of rustc becoming available in a stable release of Rust.

Implemented lints include:

  • Checking that all <a> tags have a href attribute (this is to enable screen readers to work on websites).
  • Checking that all <img> tags have an alt attribute (this is to enable screen readers to work on websites).

Possible lints include:

  • ...

Possible sources of inspiration for lints include:

Please feel free to suggest lint ideas below, or further updates on the status of the diagnostics API.

@teymour-aldridge teymour-aldridge added the feature-request A feature request label Jun 20, 2020
@teymour-aldridge
Copy link
Contributor Author

teymour-aldridge commented Jun 30, 2020

This crate might be of use in adding these errors (works on both stable and nightly).

@siku2
Copy link
Member

siku2 commented Jun 30, 2020

Just keep in mind that warnings will still only be emitted on nightly but if we use it we don't have to wait for diagnostics to hit stable.
Do you know if the crate makes it possible to suppress warnings though?

@teymour-aldridge
Copy link
Contributor Author

teymour-aldridge commented Jun 30, 2020

Do you know if the crate makes it possible to suppress warnings though?

The Diagnostics API probably has a way to do that built in. We can probably detect #[allow] attributes and filter warnings based on those.

Just keep in mind that warnings will still only be emitted on nightly

From my understanding the crate works on stable using a handcrafted API.

@siku2
Copy link
Member

siku2 commented Jun 30, 2020

The Diagnostics API probably has a way to do that built in. We can probably detect #[allow] attributes and filter warnings based on those.

The Procedural Macro Diagnostics RFC proposes support for "Lint-Associated Warnings" which works with the #[allow()] attribute but AFAIK it hasn't been implemented yet.
Since the warnings are nightly only I don't think it's a problem for them to be unsilenceable. It's just something that will need to be addressed once lint-associated warnings are implemented.


The documentation says:

Beware: warnings are nightly only, they are completely ignored on stable.

There are still a few benefits we get from using the crate:

  1. It gives us the ability to use non-abort errors. Currently an error aborts the macro compilation but there are places where it makes sense to keep going and emit all errors at the end.
  2. We can start using diagnostics right away. Even though they're useless on stable they should start working when the features are stabilized without us having to do anything.
  3. It provides a nice API for adding notes and other sections to the message.

@mc1098 mc1098 added A-yew-macro Area: The yew-macro crate feature-accepted A feature request that has been accepted and needs implementing and removed blocked labels Sep 20, 2021
@mc1098
Copy link
Contributor

mc1098 commented Sep 20, 2021

Unblocking this issue and marking the feature as accepted. I agree with @siku2 that being nightly provides an opt-in behaviour and allows us to start building up lints. Once the diagnostics API stabilises the switch, hopefully, will be relatively simply and we can hook into that API to provide ways to silence the warnings.

@mc1098 mc1098 linked a pull request Sep 25, 2021 that will close this issue
3 tasks
@mc1098 mc1098 removed a link to a pull request Sep 25, 2021
3 tasks
@ranile
Copy link
Member

ranile commented Nov 21, 2021

Now that #1748 is merged, we have main 2 areas to improve on:

  1. Use console.warn in debug mode to display warnings in browser console. This can be a temporary solution until warnings from proc-macros are stabilized.
  2. Use proper help/note messages for warnings emitted by the macro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-macro Area: The yew-macro crate feature-accepted A feature request that has been accepted and needs implementing feature-request A feature request
Projects
None yet
Development

No branches or pull requests

5 participants