Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.

Conversation

fzyzcjy
Copy link
Contributor

@fzyzcjy fzyzcjy commented Feb 24, 2022

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix
[x] New rule
[ ] Changes an existing rule
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Close #701

Is there anything you'd like reviewers to focus on?

@fzyzcjy fzyzcjy marked this pull request as draft February 24, 2022 13:05
@fzyzcjy fzyzcjy changed the title Implement avoid_collection_methods_with_unrelated_types feat: Create new rule avoid_collection_methods_with_unrelated_types Feb 24, 2022
@fzyzcjy fzyzcjy marked this pull request as ready for review February 24, 2022 14:09
@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Feb 24, 2022

Tests have passed locally :)

Copy link
Member

@incendial incendial left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fzyzcjy thank you so much for the contribution. This looks 🔥

I guess, the main questions is about supporting not only maps, but sets and lists too. What do you think?

Also, was it hard to add a rule? Maybe you have faced some problems that we should address on order to make the contribution process easier?

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Feb 25, 2022

I guess, the main questions is about supporting not only maps, but sets and lists too. What do you think?

Following the original issue, I may also add Iterable.contains.

Also, was it hard to add a rule? Maybe you have faced some problems that we should address on order to make the contribution process easier?

I just copy-and-paste-and-modify from sibling rules ;) It is ok imho

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Feb 25, 2022

@incendial Done and ready for review (again)

Maybe have a look at .md to see what is supported now

@incendial
Copy link
Member

@fzyzcjy looks amazing, thanks for addressing all the questions!

I just copy-and-paste-and-modify from sibling rules ;) It is ok imho

Thanks for answering, we don't have many people contributing new rules, so I was wondering, if it's something wrong with the flow.

@incendial
Copy link
Member

The only thing missing is the entry in changelog under Unreleased section, like feat: add static code diagnostic avoid-collection-methods-with-unrelated-types. Could you add it?

Now we'll run the rule on different public codebases in order to avoid possible edge-cases.

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Feb 26, 2022

You are welcome! Done.

@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #705 (1a5579f) into master (997dfd3) will increase coverage by 0.11%.
The diff coverage is 95.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #705      +/-   ##
==========================================
+ Coverage   86.63%   86.75%   +0.11%     
==========================================
  Files         261      263       +2     
  Lines        5551     5631      +80     
==========================================
+ Hits         4809     4885      +76     
- Misses        742      746       +4     
Impacted Files Coverage Δ
...c/analyzers/lint_analyzer/rules/rules_factory.dart 92.40% <0.00%> (-2.41%) ⬇️
lib/src/utils/dart_types_utils.dart 90.47% <90.00%> (-9.53%) ⬇️
..._collection_methods_with_unrelated_types_rule.dart 100.00% <100.00%> (ø)
...llection_methods_with_unrelated_types/visitor.dart 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 997dfd3...1a5579f. Read the comment docs.

@dkrutskikh dkrutskikh merged commit 96b976f into dart-code-checker:master Feb 26, 2022
@dkrutskikh
Copy link
Member

@fzyzcjy thank you for contribute

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Feb 26, 2022

You are welcome!

dkrutskikh pushed a commit that referenced this pull request Mar 7, 2022
…#705)

* add sccafold for new rule

* add scaffold for tests

* try to implement the rule

* add tests

* add toString to debug issues

* pass tests

* add doc

* add more functions in example

* refactor visitor to extract more generic info

* try to implement containsKey, containsValue, remove

* pass tests

* add checking `Iterable.contains`, pass tests

* revert issue.tostring

* run dart format

* refactor code

* improve doc

* try to also check subclasses of map (not finished)

* refactor dart_type_utils to avoid duplication

* make test passes

* minor change to tests

* refactor and make tests pass

* support List.remove

* change doc

* support sets

* make tests pass

* changelog
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-rules type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New rule] avoid_collection_methods_with_unrelated_types (Willing to PR)
3 participants