-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Implement issue finder for lint names #5049
Conversation
|| body.contains(&name) | ||
|| body.contains(&separated_name) | ||
|| body.contains(&dash_separated_name)) | ||
}) |
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.
how about also checking the issue labels? If at some point in the future issues are then properly labeled with a lint-*
, that could make easier to sort them
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.
Hm, we would then need over 300 labels for all of the lints and the issues had to be labeled correctly. Labeling issues by hand wouldn't be feasible. Maybe we could write an automation for such a labeling. But as long, as we don't have such a system in place, I don't think adding this to this tool makes much sense.
But The lint-*
/L-*
label idea is definitely interesting! Maybe we could add the labels of the issues in the output of this tool 🤔
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.
Maybe we could add the labels of the issues in the output of this tool
that's a great idea imo 👍
As a positive side effect, people can easily start using the right label when reporting, decreasing in time the amount of "unlabeled" linting issues.
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.
Also, but I'm not sure it makes sense, as a starting point, I'd force feed the issueing ticketing system with all the labels with a quick cURL
, so people can be instructed to use them immediately and we truncate immediately the input of "unlabeled" linting issues
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.
The thing is, that "normal" users can't label their issue by them self, but Collaborators/Members have to do this. There is also @rustbot
, but we can't check this automatically. We'd could write a GitHub Actions Workflow, that scans a new issue for a lint name and labels it accordingly.
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.
You can have issue templates that have a default label. I have some examples here: https://github.com/jyn514/rcc/tree/master/.github/ISSUE_TEMPLATE
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.
Unless I'm wrong, for issue templates to be useful we need to have the user be able to automatically apply one label out of dozens hundreds and that would require having dozens hundreds of templates, therefore a scary list of templates when the user creates the issue :-)
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.
Since we're soon completely migrated to GHA, we could implement the labeler workflow for issues and PRs, that would do that. But the question, if we want to have hundreds of labels still stands.
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.
If we can automate the label creation, I would not be against it personally but that's probably for another PR
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.
I agree. So, I'd add a check for the issue label here (because imo it's a sensible default) and then we find a way to manage the issue creation workflow.
d5d3a37
to
156dd4a
Compare
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.
Code looks good to me in general 👍 I think somewhere there should also be some instructions on how to generate the issue text. I assume it's cargo --feature issues dev issues_for_lint --name foobar
?
} | ||
|
||
fn next_link(response: &Response) -> Option<String> { | ||
if let Some(links) = response.headers().get("Link").and_then(|l| l.to_str().ok()) { |
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.
if let Some(links) = response.headers().get("Link").and_then(|l| l.to_str().ok()) { | |
if let Some(links) = response.headers().get(reqwest::headers::LINK).and_then(|l| l.to_str().ok()) { |
|
||
fn next_link(response: &Response) -> Option<String> { | ||
if let Some(links) = response.headers().get("Link").and_then(|l| l.to_str().ok()) { | ||
if let Some(cap) = NEXT_PAGE_RE.captures_iter(links).next() { |
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.
Huh, is there no way to do this without a regex?
header::AUTHORIZATION, | ||
header::HeaderValue::from_str(&format!("token {}", github_token))?, | ||
); | ||
headers.insert(header::USER_AGENT, header::HeaderValue::from_static("ghost")); |
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.
Might be nice to give the user agent a slightly more descriptive name such as 'rust-clippy issue finder'
Ok(issues) | ||
} | ||
|
||
fn filter_issues<'a>(issues: &'a [Issue], name: &str, filter: &'a [u32]) -> impl Iterator<Item = &'a Issue> { |
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.
I think it's worth adding some unit tests for this function
I think that we could really use automatic issue triage, but this probably won't lead to anything. |
Since we're going towards 1000 issues and simply don't have enough manpower to triage them properly and therefore the issue tracker gets kind of convoluted, I thought it would be a good idea to have one tracking issue for all lints. In the tracking issue, for every lint a list of issues mentioning this lint in its title or in the issue body would be displayed.
This is what this tool does. This tool is behind a feature, because it requires
reqwest
andserde
, which would otherwise blow up theclippy_dev
build.Output
absurd_extreme_comparisons
approx_constant
assign_op_pattern
blacklisted_name
block_in_if_condition_expr
block_in_if_condition_stmt
bool_comparison
bool_comparison
should not fire when testing for false #1043 (bool_comparison
should not fire when testing for false)needless_bool
triggers oncfg!
comparisons #3973 (needless_bool
triggers oncfg!
comparisons)borrow_interior_mutable_const
borrowed_box
borrowed_box
incorrectly assumes mutable boxed slice allocation won't change #2907 (borrowed_box
incorrectly assumes mutable boxed slice allocation won't change)borrowed_box
when trait object is boxed #3971 (Clippy incorrectly complains aboutborrowed_box
when trait object is boxed)box_vec
boxed_local
wasm_bindgen
#4351 (False positive for boxed local withwasm_bindgen
)cargo_common_metadata
cast_lossless
cast_possible_truncation
cast_ptr_alignment
cast_ptr_alignment
lint forpointer::cast
method #4708 (Extendcast_ptr_alignment
lint forpointer::cast
method)cast_ptr_alignment
for type alias #4754 (Extendcast_ptr_alignment
for type alias)cast_sign_loss
clone_on_copy
expl_impl_clone_on_copy
whenCopy
is manually implemented. #1254 (Disableexpl_impl_clone_on_copy
whenCopy
is manually implemented.)clone_on_copy
when the clone is immediately borrowed #1472 (False positive forclone_on_copy
when the clone is immediately borrowed)clone_on_ref_ptr
cmp_owned
cognitive_complexity
master
instead of actual version #4167 (Clippy links tomaster
instead of actual version)collapsible_if
else
after multipleelse if
#1401 (collapsible_if shouldn't warn onelse
after multipleelse if
)comparison_chain
debug_assert_with_mut_call
declare_interior_mutable_const
deprecated_cfg_attr
custom_inner_attributes
is stabilized #3660 (deprecated_cfg_attr lint: Also lint inner attributes oncecustom_inner_attributes
is stabilized)derive_hash_xor_eq
Eq
implementations #2025 (False positive in derive_hash_xor_eq with multipleEq
implementations)doc_markdown
double_comparisons
double_neg
drop_copy
drop_ref
&mut T
as well as&T
#4461 (clippy::drop_ref will trigger on&mut T
as well as&T
)else_if_without_else
empty_enum
empty_loop
no_std
#3746 (Lint: empty_loop lint onno_std
)enum_clike_unportable_variant
enum_clike_unportable_variant
#816 (False positive onenum_clike_unportable_variant
)enum_glob_use
enum_variant_names
enum_variant_names
#4639 (Incomplete postfix detected forenum_variant_names
)eq_op
erasing_op
eval_order_dependence
exit
exit
in tests #966 (lint on a call toexit
in tests)cargo clippy
code #1329 (Clean up the error reporting in thecargo clippy
code)debug_assert
#2249 (Cyclomatic Complexity should not count code indebug_assert
)serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)master
instead of actual version #4167 (Clippy links tomaster
instead of actual version)find_map
lint be more conservative #4193 (Makefind_map
lint be more conservative)p0 | ... | p_n
=> range pat #4931 (New Lint: Explicit ranges withp0 | ... | p_n
=> range pat)expect_fun_call
expect_fun_call
recommends code which is not equivalent #2928 (expect_fun_call
recommends code which is not equivalent)expl_impl_clone_on_copy
expl_impl_clone_on_copy
whenCopy
is manually implemented. #1254 (Disableexpl_impl_clone_on_copy
whenCopy
is manually implemented.)explicit_counter_loop
explicit_into_iter_loop
explicit_iter_loop
explicit_write
extend_from_slice
filter_map
filter_map
lint more specific #3188 (Make thefilter_map
lint more specific)vec.clone().into_iter().stuff().collect()
to suggestvec.iter().stuff().cloned().collect()
#3302 (lintvec.clone().into_iter().stuff().collect()
to suggestvec.iter().stuff().cloned().collect()
)find_map
lint be more conservative #4193 (Makefind_map
lint be more conservative)filter_next
find_map
find_map
lint be more conservative #4193 (Makefind_map
lint be more conservative)float_arithmetic
float_cmp
float_cmp_const
fn_to_numeric_cast
fn_to_numeric_cast_with_truncation
for_loop_over_option
for_loop_over_result
forget_ref
get_unwrap
identity_conversion
identity_conversion
name is misleading #3106 (lintidentity_conversion
name is misleading)identity_conversion
removes parenthesis with inner expression, that needs them. #4750 ([cargo fix]identity_conversion
removes parenthesis with inner expression, that needs them.)identity_op
if_let_redundant_pattern_matching
if let Some(_) / None
with methods #2353 (Do not suggest replacing simpleif let Some(_) / None
with methods)if_let_some_result
if_let_some_result
lint #4991 (Add suggestion toif_let_some_result
lint)if_same_then_else
serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)else if
blocks. #3770 (if_same_then_else Should not warn forelse if
blocks.)implicit_hasher
implicit_return
indexing_slicing
indexing_slicing
help message #2866 (Improveindexing_slicing
help message)inherent_to_string
inherent_to_string_shadow_display
inline_always
integer_arithmetic
integer_division
into_iter_on_array
into_iter_on_ref
invalid_atomic_ordering
invalid_atomic_ordering
lint to memory fence calls #5026 (Extendinvalid_atomic_ordering
lint to memory fence calls)invalid_regex
invalid_upcast_comparisons
items_after_statements
large_enum_variant
len_without_is_empty
len
andis_empty
are in differentimpl
blocks #1562 (len_without_is_empty lints iflen
andis_empty
are in differentimpl
blocks)len_zero
len_zero
with Range suggests code cannot compile #3807 (len_zero
with Range suggests code cannot compile)let_and_return
let_and_return
#3324 (Possible false positive withlet_and_return
)let_underscore_must_use
let_unit_value
linkedlist
logic_bug
logic_bug
#1916 (Renamelogic_bug
)manual_mul_add
many_single_char_names
map_entry
match_bool
match_ref_pats
match_ref_pats
suggestion. #4834 (badmatch_ref_pats
suggestion.)match_same_arms
match_same_arms
vs.&Any
#1218 (Another false positive:match_same_arms
vs.&Any
)serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)match_wild_err_arm
match_wild_err_arm
triggers weirdly #5024 (match_wild_err_arm
triggers weirdly)min_max
missing_const_for_fn
const fn
if member of member implements Drop #4979 (Wrong suggestion ofconst fn
if member of member implements Drop)missing_docs_in_private_items
missing_inline_in_public_items
mistyped_literal_suffixes
module_name_repetitions
modulo_one
multiple_inherent_impl
must_use_candidate
mut_from_ref
mut_from_ref
lint apply to unsafe functions? #4281 (Should themut_from_ref
lint apply to unsafe functions?)mut_mut
serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)mutable_key_type
mutex_atomic
mutex_atomic
#4295 (Consider removingmutex_atomic
)needless_bool
needless_bool
triggers oncfg!
comparisons #3973 (needless_bool
triggers oncfg!
comparisons)needless_borrow
serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)needless_borrow
was temporarily moved tonursery
in April 2018 #3742 (needless_borrow
was temporarily moved tonursery
in April 2018)needless_continue
needless_doctest_main
needless_lifetimes
serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)needless_pass_by_value
needless_range_loop
needless_range_loop
could suggest usingclone_from_slice
in some cases #1831 (needless_range_loop
could suggest usingclone_from_slice
in some cases)needless_range_loop
#1864 (Improve suggestions forneedless_range_loop
)needless_return
cargo clippy --all-targets
issues diagnostics for library crate twice #4092 (cargo clippy --all-targets
issues diagnostics for library crate twice)neg_multiply
new_ret_no_self
new_ret_no_self
if self type carries lifetime annotation? #734 (False positive innew_ret_no_self
if self type carries lifetime annotation?)new_without_default
new_without_default
is shown for structs with private fields? #1878 (Whynew_without_default
is shown for structs with private fields?)new_without_default_derive
triggers on types which cannot#[derive(Default)]
#1890 (new_without_default_derive
triggers on types which cannot#[derive(Default)]
)derive
is already applied to struct, suggest extending it, not replacing it #3053 (Whenderive
is already applied to struct, suggest extending it, not replacing it)no_effect
non_ascii_literal
nonminimal_bool
!any(==)
and similar #1334 (Lint!any(==)
and similar)not_unsafe_ptr_arg_deref
op_ref
&mut T
as well as&T
#4461 (clippy::drop_ref will trigger on&mut T
as well as&T
)option_and_then_some
result_and_then_ok
lint #4412 (Addresult_and_then_ok
lint)option_map_unit_fn
option_map_unwrap_or
Option::map
call with an "identity" #3236 (Why does Clippy not suggest removing anOption::map
call with an "identity")option_map_unwrap_or_else
Option::map
call with an "identity" #3236 (Why does Clippy not suggest removing anOption::map
call with an "identity")option_option
option_unwrap_used
or_fun_call
or_fun_call
#1609 (False positive foror_fun_call
)out_of_bounds_indexing
overflow_check_conditional
a - b > b
is not really an underflow check #2457 (false positive:a - b > b
is not really an underflow check)panic
cargo clippy
code #1329 (Clean up the error reporting in thecargo clippy
code)tcx.hir.body
without accompaningtcx.body_tables
#1976 (Lint usages oftcx.hir.body
without accompaningtcx.body_tables
)panic!
being used #2297 (Add a lint forpanic!
being used)indexing_slicing
help message #2866 (Improveindexing_slicing
help message)expect_fun_call
recommends code which is not equivalent #2928 (expect_fun_call
recommends code which is not equivalent)Any
pitfalls #2988 (Any
pitfalls)#[no_mangle]
functions that don't have#[inline(never)]
#3070 (Lint calls to#[no_mangle]
functions that don't have#[inline(never)]
)serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)no_std
#3746 (Lint: empty_loop lint onno_std
)mut_from_ref
lint apply to unsafe functions? #4281 (Should themut_from_ref
lint apply to unsafe functions?)match_wild_err_arm
triggers weirdly #5024 (match_wild_err_arm
triggers weirdly)invalid_atomic_ordering
lint to memory fence calls #5026 (Extendinvalid_atomic_ordering
lint to memory fence calls)partialeq_ne_impl
possible_missing_comma
precedence
print_literal
cargo clippy
does not process build scripts #2802 (cargo clippy
does not process build scripts)print_with_newline
ptr_arg
ptr_arg
lint #1981 (Typedefs andptr_arg
lint)question_mark
range_plus_one
redundant_clone
redundant_closure
redundant_closure_call
redundant_field_names
redundant_pattern
if let Some(_) / None
with methods #2353 (Do not suggest replacing simpleif let Some(_) / None
with methods)redundant_pattern_matching
if let Some(_) / None
with methods #2353 (Do not suggest replacing simpleif let Some(_) / None
with methods)redundant_static_lifetimes
regex_macro
replace_consts
replace_consts
enabled by default? #2380 (Why isreplace_consts
enabled by default?)result_map_unwrap_or_else
result_unwrap_used
reverse_range_loop
shadow_reuse
shadow_same
shadow_unrelated
should_assert_eq
should_implement_trait
similar_names
single_char_pattern
SINGLE_CHAR_PATTERN
check for bounds, notself
type #871 (MakeSINGLE_CHAR_PATTERN
check for bounds, notself
type)single_match
if
when possible #173 (single_match should suggest plainif
when possible)serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)single_match_else
serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)str_to_string
string_add
string_add_assign
string_lit_as_bytes
string_lit_as_bytes
lint #4494 (Incorrect suggestion ofstring_lit_as_bytes
lint)string_to_string
suspicious_arithmetic_impl
suspicious_else_formatting
suspicious_map
suspicious_map
#4793 (Improve documentation forsuspicious_map
)temporary_cstring_as_ptr
todo
if let
instead of nestedif let { if let
#2521 (Suggestion: Singleif let
instead of nestedif let { if let
)too_many_arguments
too_many_arguments
should ignore functions callednew
#1576 (too_many_arguments
should ignore functions callednew
)toplevel_ref_arg
ref
on an entirelet
pattern is discouraged" lint #1882 (Wrong code example for the "ref
on an entirelet
pattern is discouraged" lint)mut
to the right-hand side #3110 (Suggestions for toplevel_ref_arg should not movemut
to the right-hand side)transmute_ptr_to_ptr
transmute()
with raw trait object pointers to change trait object lifetimes #2906 (False positives when usingtransmute()
with raw trait object pointers to change trait object lifetimes)transmute_ptr_to_ref
transmute()
with raw trait object pointers to change trait object lifetimes #2906 (False positives when usingtransmute()
with raw trait object pointers to change trait object lifetimes)trivial_regex
trivial_regex
false positive and/or incorrect hint forreplace
? #1943 (trivial_regex
false positive and/or incorrect hint forreplace
?)trivially_copy_pass_by_ref
type_complexity
type_repetition_in_bounds
unimplemented
..
binding in struct #638 (unnecessary..
binding in struct)Result
s only used in println!() #2529 (Warn aboutResult
s only used in println!())uninit_assumed_init
unit_arg
unit_arg
removes calls to functions with side-effects #4741 ([cargo fix]unit_arg
removes calls to functions with side-effects)unit_cmp
unnecessary_filter_map
unnecessary_fold
unnecessary_mut_passed
unnecessary_unwrap
is_some
is just part of the conditional #4530 (unnecessary_unwrap: should not trigger if theis_some
is just part of the conditional)unneeded_field_pattern
unreachable
!
/Infallibe
in trait implementations with a result type. #4644 (Suggest using!
/Infallibe
in trait implementations with a result type.)unreadable_literal
unused_io_amount
Read::read_exact
inside implementations ofRead::read_exact
that don't directly use the output ofRead::read
#4836 (Clippy errors and says to useRead::read_exact
inside implementations ofRead::read_exact
that don't directly use the output ofRead::read
)unused_label
unused_self
unused_self
triggers if only the lifetime ofself
is used #4928 (FP:unused_self
triggers if only the lifetime ofself
is used)unused_unit
use_debug
use_debug
lint should not trigger ondebug!()
log macro calls #2132 (use_debug
lint should not trigger ondebug!()
log macro calls)use_self
use_self
when using an associated type instead of the type itself. #2843 (False positive foruse_self
when using an associated type instead of the type itself.)From
conversion for theBox<dyn Trait>
. #4305 (Suggestion does not compile: use self with when implementingFrom
conversion for theBox<dyn Trait>
.)use_self
when using core::mem::transmute in From impl #4734 (False positive foruse_self
when using core::mem::transmute in From impl)used_underscore_binding
used_underscore_binding
#947 (Fixused_underscore_binding
)serde_derive
" in testused_underscore_binding_macro
#3643 ("multiple matching crates forserde_derive
" in testused_underscore_binding_macro
)useless_attribute
useless_attribute
lint #4467 (Incorrect suggestion inuseless_attribute
lint)useless_format
useless_format
to lint values other than&str
andString
#3753 (Extenduseless_format
to lint values other than&str
andString
)useless_let_if_seq
useless_let_if_seq
lint #2918 (Incorrect suggestion byuseless_let_if_seq
lint)useless_let_if_seq
when having side effects. #4124 (Bad suggestion ofuseless_let_if_seq
when having side effects.)useless_let_if_seq
lint #4448 (Suggest binding to a tuple if possible in theuseless_let_if_seq
lint)useless_transmute
transmute()
with raw trait object pointers to change trait object lifetimes #2906 (False positives when usingtransmute()
with raw trait object pointers to change trait object lifetimes)useless_vec
vec_box
while_immutable_condition
while_let_loop
while_let_loop
fails to account for lifetimes #362 (while_let_loop
fails to account for lifetimes)while_let_on_iterator
while_let_on_iterator
suggestion when pattern is refutable #3780 (Badwhile_let_on_iterator
suggestion when pattern is refutable)wrong_pub_self_convention
wrong_self_convention
wrong_self_convention
does not recognizethis: Self
#3414 (wrong_self_convention
does not recognizethis: Self
)into_*
#4037 (wrong_self_convention false positive for async fninto_*
)zero_prefixed_literal
zero_ptr
I'm pretty satisfied with the output. Some lint names that are too generic, like
panic
orexit
, need some extra work though.Usage:
Your thoughts about having such a tracking issue?
changelog: none