Fix some suggestions for redundant_pattern_matching
.. and change the Applicability to `MaybeIncorrect`.
Fixes the problem displayed in https://github.com/rust-lang/rust-clippy/issues/4344#issuecomment-519206388.
We now append `{}` to the suggestion so that the conditional has the
correct syntax again.
(If we were to _remove_ the `if` instead, it would trigger the
`unused_must_use` warning for `#[must_use]` types.)
changelog: Fix some suggestions for `redundant_pattern_matching`
Fixes the problem displayed in https://github.com/rust-lang/rust-clippy/issues/4344#issuecomment-519206388.
We now append `{}` to the suggestion so that the conditional has the
correct syntax again.
(If we were to _remove_ the `if` instead, it would trigger the
`unused_must_use` warning for `#[must_use]` types.
Add autofixable suggestion for unseparated integer literal suffixes
changelog: Add autofixable suggestion for unseparated integer literal suffixes
Somewhat WIP, since I haven't been able to get this working when adding `// run-rustfix` to `ui/literals.rs`. I think the issue is that there are multiple suggestions operating on one numerical literal, and I'm not sure what the best approach is to work around that.
Thanks
Update lint deprecation for tool lints
changelog: Allow tool lints (`clippy::*`) to be deprecated
Our lint deprecation previously didn't work for tool lints, because
`register_removed` was registering lints to be removed _without_ the
`clippy` prefix.
Fixes#4349
Our lint deprecation previously didn't work for tool lints, because
`register_removed` was registering lints to be removed _without_ the
`clippy` prefix.
new_ret_no_self: allow Self in inner type for impl Trait return types
Check the inner types of associated types of a trait when checking for Self in the return type of a `new` method. This means that the following will no longer warn:
```rust
trait Trait {
type Inner;
}
struct S;
impl S {
fn new() -> impl Trait<Inner = Option<Self>> {
struct TraitImpl;
impl Trait for TraitImpl {
type Inner = Option<S>;
}
TraitImpl
}
}
```
```rust
#![feature(async_await)]
struct Connection;
impl Connection {
async fn new() -> Result<Self, ()> {
Ok(S)
}
}
```
closes#4359
changelog: fix `new_ret_no_self` lint for async `new` functions.
Add run-rustfix for needless_bool lint
This splits up the needless_bool tests into `fixable.rs` and
`simple.rs`. `simple.rs` contains the code that triggers the lint
diagnostic without a suggestion.
changelog: none
cc #3630
This splits up the needless_bool tests into `fixable.rs` and
`simple.rs`. `simple.rs` contains the code that triggers the lint
diagnostic without a suggestion.
Split up cast.rs tests, run-rustfix for unnecessary_cast
This splits up the cast.rs tests and enables rustfix tests for the part
of the `unnecessary_cast` lint that emits `MachineApplicable`
suggestions.
changelog: none
cc #3630
This splits up the cast.rs tests and enables rustfix tests for the part
of the `unnecessary_cast` lint that emits `MachineApplicable`
suggestions.
cc #3630
Add negation to `len_zero` lint to show more explicit message.
Fixes#4304 I have updated the `len_zero` to show the required negation in case of like the below case.
```
fn main() {
let v = vec![1];
if v.len() > 0 {
}
}
```
changelog: Clarify suggestion of `len_zero` lint.
UI Test Cleanup: No wrong_self_convention in methods.rs
These cases are already covered in `tests/ui/wrong_self_convention.rs`.
cc #2038
changelog: none
Don't nudge people towards toilet closures when producing owl results
`.map_err(drop)` should never be linted since sometimes you want to produce `Result<(), ()>` and the alternative is `.map_err(|_| ())`, which can be ugly. We don't seem to, but it's good to specifically test for this.
changelog: none
r? @yaahallo
trait bounds lint - repeated types
This PR is to tackle https://github.com/rust-lang/rust-clippy/issues/3764 it's still a WIP and doesn't work but this is an initial stab. It builds though I haven't added any tests as I'm not sure where lint tests should go?
Unfortunately, it seems id isn't tied to the type itself but I guess where it is in the AST? Looking at https://manishearth.github.io/rust-internals-docs/syntax/ast/struct.Ty.html I can't see any members that would let me tell if a type was repeated in multiple trait bounds.
There may be other issues with how I've implemented this so any assistance is appreciated!
changelog: Add new lint: `type_repetition_in_bounds`
This lint adds warning if types are redundantly repeated in trait bounds i.e. `T: Copy, T: Clone` instead of `T: Copy + Clone`. This is a late pass trait lint and has necessitated the addition of code to allow hashing of TyKinds without taking into account Span information.
Ignore generated fresh lifetimes in elision check
<!--
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.
If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR -->
fixes#3988
changelog: Ignore generated fresh lifetimes in elision check.
**HELP**: It seems `tests/ui` are compiled under edition 2015, and I don't know how to add tests for this properly.
Here is the test input it had already passed:
```rust
#![feature(async_await)]
#![allow(dead_code)]
async fn sink1<'a>(_: &'a str) {} // lint
async fn sink1_elided(_: &str) {} // ok
async fn one_to_one<'a>(s: &'a str) -> &'a str { s } // lint
async fn one_to_one_elided(s: &str) -> &str { s } // ok
async fn all_to_one<'a>(a: &'a str, _b: &'a str) -> &'a str { a } // ok
// async fn unrelated(_: &str, _: &str) {} // Not allowed in async fn
// #3988
struct Foo;
impl Foo {
pub async fn foo(&mut self) {} // ok
}
// rust-lang/rust#61115
async fn print(s: &str) { // ok
println!("{}", s);
}
fn main() {}
```
Update pulldown-cmark to 0.5.3
Fixes a couple of crashes of which I added one example to our tests.
changelog: Update `pulldown-cmark` to 0.5.3 to fix potential crashes in `doc_markdown` lint
false positives fixes of `implicit_return`
- Handle returning macro statements properly (remove "this error originates in a macro outside of the current crate")
- Handle functions that return never type
- Handle functions that panic but do not return never type
changelog: Fix false positives in `implicit_return` lint pertaining to macros and panics
Implement lint for inherent to_string() method.
Fixes#4247
changelog: Implement two new lints: `inherent_to_string` and `inherent_to_string_shadow_display`
1) Emits a warning if a type implements an inherent method `to_string(&self) -> String`
2) Emits an error if a type implements an inherent method `to_string(&self) -> String` and also implements the `Display` trait
UI Test Cleanup: Split up checked_unwrap tests
Let's slowly bring that ticket closer to the finish line 🐌🏁
This splits up `tests/ui/checked_unwrap.rs` into:
* `tests/ui/checked_unwrap/complex.rs`
* `tests/ui/checked_unwrap/simple.rs`
Based on the naming of the methods in the tests.
changelog: none
cc #2038
This splits up `tests/ui/checked_unwrap.rs` into:
* `tests/ui/checked_unwrap/complex.rs`
* `tests/ui/checked_unwrap/simple.rs`
Based on the naming of the methods in the tests.
cc #2038
Improve cast_ptr_alignment lint
<!--
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.
If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- [x] Followed [lint naming conventions][lint_naming]
- [x] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Executed `util/dev update_lints`
- [x] Added lint documentation
- [x] Run `cargo fmt`
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR -->
* print alignment in bytes in the lint message
* ignore ZST left-hand types
Fixes#3797 and #4256
changelog:
* `cast_ptr_alignment`: Print alignment in bytes in the lint message
* `cast_ptr_alignment`: Ignore casting from ZST left-hand types
Avoid reporting string_lit_as_bytes for long strings
Port of @jens1o code ([b76f939][jens1o_commit])
Fixes#1208
[jens1o_commit]: b76f939ac2
<!--
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.
If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR -->
changelog: bugfix for long strings as bytes
This is in preperation for https://github.com/rust-lang/rust/pull/62150
Also allows deprecations in preperations for intrinsics::init being deprecated, which is a planned future step.
lint does not trigger when there is a difference in mutability
lint does not trigger when the method belongs to a trait which is not implemebted directly (Deref)
trivially_copy_pass_by_ref: print size of type and limit in the lint message
changelog: trivially_copy_pass_by_ref: print size of type and limit in the lint message
Fix match_same_arms to fail late
Changes:
- Add a function search_same_list which return a list of matched expressions
- Change the match_same_arms implementation behavior. It will lint each same arms found.
fixes#4096
changelog: none
Implement "Use last" lint
Closes#3673
This lint checks the use of `x.get(x.len() - 1)` and suggests `x.last()` (where `x` is a `Vec`).
There's at least one more thing that needs to happen here. I believe I am correctly checking most of the scenarios and avoiding false positives. However, if different `Vec`s are used for the `x.get` and `y.len`, then it will emit a false positive. In other words, I need to check to make sure the `Vec`s are the same.
Also, a lot of these commits were temporary and not helpful to the project history...am I supposed to squash on merge? If so, how do I do that?
changelog: New lint: `get_last_with_len`
* Removes an unneeded `#![warn(clippy::extra_unused_lifetimes)]` from
the `needless_lifetimes` test - the tests for those are in a different
file.
* Renames `unused_lt.{rs,stderr}` to `extra_unused_lifetimes.{rs,stderr}`
* Renames `lifetimes.{rs,stderr}` to `needless_lifetimes.{rs,stderr}`
Added lint for TryFrom for checked integer conversion.
works towards #3947
Added lint for try_from for checked integer conversion.
Should recognize simple & straight-forward checked integer conversions.
Move the method checking into a new lint called
`redundant_closures_for_method_calls` and put it in the pedantic group.
This aspect of the lint seems more controversial than the rest.
cc #3942
At some stage, we started adding test cases inside the function which tests
refutable patterns. This commits splits those test cases out into separate
functions.
Fix#4033 search_is_some
Fixes#4033.
Suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()` (Lint [search_is_some](https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some))
FnDecl of `find`:
```rust
fn find<P>(&mut self, mut p: P) -> Option<Self::Item> where
P: FnMut(&Self::Item) -> bool
```
FnDecl of `any`:
```rust
fn any<F>(&mut self, mut f: F) -> bool where
F: FnMut(Self::Item) -> bool
```
If match on `|&_|` in closure of `find`, only use `|_|` in the suggestion.
PS. It's the first time that I have used the `hir` API, please correct me if there is any mistake 😺
useless_let_if_seq handle interior mutability
fixes#3043
This passes all tests, including a new one specifically dealing with a type with interior mutability. The main thing I'm unsure of is whether the span I used in the call to `is_freeze` is the most appropriate span to use, or if it matters.
Do not trigger redundant_closure for non-function types
fixes#3898
Added a check for the entity being called in the closure body to be a FnDef. This way lint does not trigger for ADTs (Box) but I'm not sure if it's correct and not too restrictive.
<!--
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.
If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR -->
changelog: Fix false positive in `redundant_closure` pertaining to non-function types
Ignore non-const ctor expressions in or_fn_call
Fixes https://github.com/rust-lang/rust-clippy/issues/1338
Should have been fixed by #919, however that focuses on const ctor expressions only, and `.or(Some(local))` isn't const.
This also automatically ignores things like `.or(Some(local.clone())` which we don't actually want to do; I need to figure out what to do here.
changelog: Fixed false positive in [`or_fn_call`] pertaining to enum variant constructors
r? @oli-obk @phansch
Allow allowing of toplevel_ref_arg lint
I'm not sure why some lints need the `HirId` to be able to recognize the
lint level attributes, but this commit makes the lint level attributes
work for `toplevel_ref_arg`.
Fixes#2332
changelog: Allow allowing of `toplevel_ref_arg` lint
Add run-rustfix for match_as_ref lint
* Extracts `match_as_ref` into separate file
* Adds `// run-rustfix` to `tests/ui/match_as_ref.rs`
cc #3630
changelog: none
Fix false positive in module_name_repetitions lint
This lint was triggering on modules inside expanded attrs, like
for example `#[cfg(test)]` and possibly more.
It was not reporting a location in #3892 because `span.lo()` and `span.hi()` both were 0.
Fixes#3892
changelog: Fix false positive in `module_name_repetitions` lint
I'm not sure why some lints need the `HirId` to be able to recognize the
lint level attributes, but this commit makes the lint level attributes
work for `toplevel_ref_arg`.
Add Lint PathBufPushOverwrite
Closes#3923
This is a very simple Lint that checks if push is being called with a Root Path. Because that can make it overwrite the previous path.
I used std::path::Path to check if it's root, in order to keep it working across windows/linux environments instead of checking for '/'. Is that alright?
On the `if_chain!` block, Is there a way to make it short while getting the value of the first argument? I got the example from other lints.
Note that this is first Lint, I hope I got everything covered 🚀
Add run-rustfix for option_map_or_none lint
* Extracts `option_map_or_none` tests into separate file
* Adds `// run-rustfix` to `tests/ui/option_map_or_none.rs`
cc #3630
Add run-rustfix for deref_addrof lint
* renames `tests/ui/reference.{rs,stderr}` to
`tests/ui/deref_addrof.{rs,stderr}
* Moves small part of the testfile to a separate file as the lint
triggered again on the fixed code (as intended)
* Adds `// run-rustfix` to `tests/ui/deref_addrof.rs`
cc #3630
* renames `tests/ui/reference.{rs,stderr}` to
`tests/ui/deref_addrof.{rs,stderr}
* Moves small part of the testfile to a separate file as the lint
triggered again on the fixed code (as intended)
* Adds `// run-rustfix` to `tests/ui/deref_addrof.rs`
Fix ICE in decimal_literal_representation lint
Handling the integer parsing properly instead of just unwrapping.
Note that the test is not catching the ICE because plain UI tests
[currently hide ICEs][compiletest_issue]. Once that issue is fixed, this
test would fail properly again.
Fixes#3891
[compiletest_issue]: https://github.com/laumann/compiletest-rs/issues/169
Handling the integer parsing properly instead of just unwrapping.
Note that the test is not catching the ICE because plain UI tests
[currently hide ICEs][compiletest_issue]. Once that issue is fixed, this
test would fail properly again.
[compiletest_issue]: https://github.com/laumann/compiletest-rs/issues/169
Fix `explicit_counter_loop` suggestion
#1670
This code seems to me to work, but I have two question.
* Because range expression desugared in hir, `Sugg::hir` doesn't add parenthesis to range expression. Which function is better to check range do you think, `check_for_loop_explicit_counter` or `hir_from_snippet`?
* Do you think we need to distinguish between range expression and struct expression that creates `std::ops::Range*`?
* Late Lint pass, catches:
* One liner: 0 -> null -> transmute
* One liner: std:null() -> transmute
* Const (which resolves to null) -> transmute
* UI Test case for Lint
* Updated test for issue 3849, because now the lint that code generated is in Clippy.
* Expanded `const.rs` miri-based Constant Folding code, to cover
raw pointers
The moved tests were part of `tests/ui/literals.rs` before. Now they are
placed in `tests/ui/unreadable_literal.rs`, which now also runs
`rustfix` on these 4 extra let statements.
Add `doc(include = ...)` detection to `missing_docs_in_private_items`
The whole `missing documentation in crate` part doesn't have any tests. If I should add test cases tell me.
Run rustfix for string_lit_as_bytes tests
This moves the `string_lit_as_bytes` tests into a new file and enables
rustfix tests for them.
cc #3603, #2038
Fix `boxed_local` suggestion
Don't warn about an argument that is moved into a closure.
ExprUseVisitor doesn't walk into nested bodies so use a new
visitor that collects the variables that are moved into closures.
Fixes#3739
* Ran automatic naming update
* Formalized rename of `cyclomatic_complexity` to `cognitive_complexity`
** Added the rename to `lib.rs`
** Added rename test
* Added warning for deprecated key `cyclomatic_complexity_threshold` and tests for it
* Added deprecation status for Clippy's builtin attribute
* Updated tests for new builtin attribute renaming
Fix `bool_comparison` with non-`bool` expressions
Fixes#3703.
It just moves around the type check that was already there for some comparison to all of them, because if one type isn't `bool`, none of those comparison can be simplified.
Fix ICE #3719+#3718 in lint match_ref_pats
Fixes#3719
This conveniently also fixes#3718
The ICE occurs when the match expression was a macro call, where the macro was defined in another file. Since we don't have the ability to reproduce this behavior with our UI tests (AFAIK), I couldn't add a test reproducing this ICE.. However, I added a test which is related to the ICE, to show the new behavior of the lint.
I tested it with the mscheme repo locally and the ICE didn't happen anymore.
r? @matthiaskrgr
Fix ICE #3747
I'm not sure if this was the correct approach.
I don't know if I put tests/ui/crashses/ice-3747.rs in correct place because the test always passed when I ran it with `cargo test`, even without the fix applied.
If I run that test with `env CLIPPY_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug tests/ui/crashes/ice-3747.rs` then the test correctly fails without the fix applied
fixes#3747
Many people run rustfmt automatically on save. Format-dependent tests
should be marked with `#[rustfmt::skip]` to prevent accidental
reformatting from this. As a bonus the rest of the code can the formatted.
Make needless_range_loop not applicable to structures without iter method
Fixes https://github.com/rust-lang/rust-clippy/issues/3788
Now we will start lint indexed structure only if it has known iter or iter_mut method implemented.
Don't warn about an argument that is moved into a closure.
ExprUseVisitor doesn't walk into nested bodies so use a new
visitor that collects the variables that are moved into closures.
Fixes#3739
Don't check [print/write]_with_newline on raw strings
Some tests for #3778 and some maybe-not-the-greatest code that passes those tests!
I didn't run `fmt` because a) it doesn't seem to install on nightly for me, and b) on stable it wanted to apply formatting to over 90 files. Happy to make any tweaks though!
I suspect this contribution may require more than just tweaks. I'm still sort of new to rust so it may not be idiomatic, and the specific approach I took feels a little heavy-handed and brittle. I'm happy to make changes with some guidance, or equally happy if this gives a starting place for someone else to do it better :)
This test doesn't reproduce the ICE since it only happens, when the macro is defined in another file.
Currently we can't add tests with multiple files AFAIK
Also using the auxiliary folder didn't help
Add a lint to warn on `T: Drop` bounds
**What it does:** Checks for generics with `std::ops::Drop` as bounds.
**Why is this bad?** `Drop` bounds do not really accomplish anything.
A type may have compiler-generated drop glue without implementing the
`Drop` trait itself. The `Drop` trait also only has one method,
`Drop::drop`, and that function is by fiat not callable in user code.
So there is really no use case for using `Drop` in trait bounds.
**Known problems:** None.
**Example:**
```rust
fn foo<T: Drop>() {}
```
Fixes#3773
Both regular strings and raw strings can contain literal newlines. This commit
extends the lint to also warn about terminating strings with these.
Behaviour handling for raw strings is also moved into `check_newlines` by
passing in the `is_raw` boolean from `check_tts` as
[suggested](https://github.com/rust-lang/rust-clippy/pull/3781#pullrequestreview-204663732)
Literal `\n` characters (not a newline) in a `r"raw"` string should not
fail the lint.
This affects both write_with_newline and print_with_newline, so it is added in
both places.
I also copied a missing test case from write_with_newline over to
print_with_newline and added a note that one of those tests is supposed to
fail.
**What it does:** Checks for generics with `std::ops::Drop` as bounds.
**Why is this bad?** `Drop` bounds do not really accomplish anything.
A type may have compiler-generated drop glue without implementing the
`Drop` trait itself. The `Drop` trait also only has one method,
`Drop::drop`, and that function is by fiat not callable in user code.
So there is really no use case for using `Drop` in trait bounds.
**Known problems:** None.
**Example:**
```rust
fn foo<T: Drop>() {}
```
Fix ICE in needless_pass_by_value lint
If I understand it correctly, we were first creating a type with a
`RegionKind::ReErased` region and then deleted it again in
`util::implements_trait` with:
cx.tcx.erase_regions(&ty);
causing the type query to fail.
It looks like using `ReEmpty` works around that deletion.
Fixes#3144
Macro check for assertion_on_constants lint
The `assertion_on_constants` lint currently has following output for this code [Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6f2c9df6fc50baf847212d3b5136ee97):
```rust
macro_rules! assert_const {
($len:expr) => {
assert!($len > 0);
}
}
fn main() {
assert_const!(3);
assert_const!(-1);
}
```
```
warning: assert!(const: true) will be optimized out by the compiler
--> src/main.rs:3:9
|
3 | assert!($len > 0);
| ^^^^^^^^^^^^^^^^^^
...
8 | assert_const!(3);
| ---------------- in this macro invocation
|
= note: #[warn(clippy::assertions_on_constants)] on by default
= help: remove it
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
warning: assert!(const: false) should probably be replaced
--> src/main.rs:3:9
|
3 | assert!($len > 0);
| ^^^^^^^^^^^^^^^^^^
...
9 | assert_const!(-1);
| ----------------- in this macro invocation
|
= help: use panic!() or unreachable!()
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
```
This is contradictory. This lint should not trigger if the `assert!` is in a macro itself.
If I understand it correctly, we were first creating a type with a
`RegionKind::ReErased` region and then deleted it again in
`util::implements_trait` with:
cx.tcx.erase_regions(&ty);
causing the type query to fail.
It looks like using `ReEmpty` works around that deletion.
Fix `cast_sign_loss` false positive
This checks if the value is a non-negative constant before linting about
losing the sign.
Because the `constant` function doesn't handle const functions, we check if
the value is from a call to a `max_value` function directly. A utility method
called `get_def_path` was added to make checking for the function paths
easier.
Fixes#2728
Adding lint test for excessive LOC.
This is a WIP for #2377. Just wanted to pull in because I had a few questions:
1. Is it okay that I'm approaching this via counting by looking at each line in the snippet instead of looking at the AST tree? If there's another way to do it, I want to make sure I'm doing the correct way, but I wasn't sure since the output AST JSON doesn't seem to contain whitespace.
2. My function is definitely going to trigger the lint, so also wanted to see if there was something obvious I could do to reduce it.
3. Are the two tests fine, or is there something obvious I'm missing?
4. Obviously bigger question - am I approaching the line count correctly. Current strategy is count a line if it contains some code, so skip if it's just comments or empty.
