-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Get the block content from the proper context #15014
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
Conversation
&& let ctxt = expr.span.ctxt() | ||
&& then_expr.span.ctxt() == ctxt | ||
&& !expr.span.from_expansion() | ||
&& !then_expr.span.from_expansion() | ||
&& is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome) | ||
&& is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone) | ||
&& !is_else_clause(cx.tcx, expr) | ||
&& !is_in_const_context(cx) | ||
&& !expr.span.in_external_macro(cx.sess().source_map()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for why we don't want to lint code from within local macros anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. Is there a context in which I can get the non-expanded code? If the if … { Some(…) } else { None }
is in the macro, using the context of the if
expression is not enough to get non-expanded code for the condition or the content of the Some
. How can I retrieve it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another reason would be that we sometimes suggest .then(|| …)
and sometimes .then_some()
. Here, in a macro, we would have to suggest .then(|| …)
because we won't check every call for the absence of side effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, those are fair points.
The first statement of the block might have been in a different context from the expression. Walk up to the right context to get bounds properly. Also, switch to `snippet_with_applicability()` since we know that we are in the right context already.
22d9980
to
bd33a02
Compare
Rebased |
Lintcheck changes for bd33a02
This comment will be updated if you push new changes |
&& let ctxt = expr.span.ctxt() | ||
&& then_expr.span.ctxt() == ctxt | ||
&& !expr.span.from_expansion() | ||
&& !then_expr.span.from_expansion() | ||
&& is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome) | ||
&& is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone) | ||
&& !is_else_clause(cx.tcx, expr) | ||
&& !is_in_const_context(cx) | ||
&& !expr.span.in_external_macro(cx.sess().source_map()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, those are fair points.
The first statement of the block might have been in a different context from the expression. Walk up to the right context to get bounds properly.
Also, switch to
snippet_with_applicability()
since we know that we are in the right context already.changelog: [
if_then_some_else_none
]: emit well-formed suggestion, and do not lint inside macrosFixes #15005