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.

Sign up for GitHub

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

Jump to bottom

Inline regexps defined in analyzer util.dart. #769

Closed
pq opened this issue Aug 3, 2017 · 2 comments · Fixed by #770
Closed

Inline regexps defined in analyzer util.dart. #769

pq opened this issue Aug 3, 2017 · 2 comments · Fixed by #770
Assignees
@pq
Labels
type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable
Milestone
0.1.36

Comments

@pq
Copy link
Member

pq commented Aug 3, 2017

ATM we depend on regexps that are defined in the analyzer package for some of our lints. This makes changing them hard (e.g., #684) and doesn't add any real benefit (IMO); also note that the tests live in the linter while the definitions are in the analyzer --- another odd coupling!
@pq pq added the type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable label Aug 3, 2017
@pq pq added this to the 0.1.36 milestone Aug 3, 2017
@pq pq self-assigned this Aug 3, 2017
@bwilkerson
Copy link
Member

... and doesn't add any real benefit ...

The (possibly only theoretical) benefit is that these utility methods are available for all clients of the analyzer code base, not just the lint rules. Given the number of clients, it seems quite likely that these would be of use to more than the lint rules.

If you do pull them back out of the analyzer, note that this is a (potentially) breaking change and that you ought to put some effort into making sure that there are no other users.

... the tests live in the linter while the definitions are in the analyzer ...

Well, that's just wrong! The tests of the utility methods need to be moved into analyzer (assuming that the utility methods aren't removed from analyzer, of course).

@pq
Copy link
Member Author

pq commented Aug 3, 2017

As per our discussion, deprecating (but not removing) analyzer utility methods:

https://codereview.chromium.org/2989363002/

pq added a commit that referenced this issue Aug 3, 2017
@pq
Update library prefix matcher; inline APIs (#684,#769).
09a2b5a 
Fixes: #684
Fixes: #769
pq added a commit to dart-lang/sdk that referenced this issue Aug 3, 2017
@pq
Deprecate unintended APIs.
1c5101e 
R=brianwilkerson@google.com

See: dart-lang/linter#769
Review-Url: https://codereview.chromium.org/2989363002 .
@pq pq mentioned this issue Aug 3, 2017
Merged
@pq pq closed this as completed in #770 Aug 3, 2017
pq added a commit that referenced this issue Aug 3, 2017
@pq
Update library prefix matcher; inline APIs (#684,#769). (#770)
9f0af60 
Fixes: #684
Fixes: #769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pq pq

Labels
type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable
Projects
None yet
Milestone
0.1.36
Development

Successfully merging a pull request may close this issue.

Update library prefix matcher; inline APIs (#684,#769). dart-lang/linter
2 participants
@pq @bwilkerson