Skip to content

Enable unreachable_from_main lint - it is stable now!!1 #129854

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
Merged
auto-submit merged 2 commits into from
Jul 6, 2023
Merged

Enable unreachable_from_main lint - it is stable now!!1 #129854

auto-submit merged 2 commits into from
Jul 6, 2023

Conversation

@goderbauer
Copy link
Member

@goderbauer goderbauer commented Jun 30, 2023 •
edited
Loading

PLUS: clean-up of all the unreachable stuff.

@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: scrolling Viewports, list views, slivers, etc. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: routes Navigator, Router, and related APIs. documentation labels Jun 30, 2023
@goderbauer goderbauer force-pushed the unreachable_from_main branch 2 times, most recently from 4a0bbc3 to 3c10976 Compare June 30, 2023 21:52
@christopherfujino
Copy link
Contributor

christopherfujino commented Jun 30, 2023

can you link some more references to this black magic? how does it know which main() to start from? or does it just evaluate every function named main() in a library in the package?

@goderbauer
Copy link
Member Author

goderbauer commented Jun 30, 2023

It only considers files that have a main function. And for those files its checks if everything in that file is reachable from that main. Its mostly targeted towards tests, which always have a main function and where people often declare everything as public. Then, when they refactor the test and no longer need the public thing, they forget to delete it.

@christopherfujino
Copy link
Contributor

christopherfujino commented Jun 30, 2023

It only considers files that have a main function. And for those files its checks if everything in that file is reachable from that main. Its mostly targeted towards tests, which always have a main function and where people often declare everything as public. Then, when they refactor the test and no longer need the public thing, they forget to delete it.

Ahh, ok, that's much less magical than I thought. Nice cleanup.

christopherfujino
christopherfujino approved these changes Jun 30, 2023
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

Tools LGTM

srawlins
srawlins approved these changes Jul 3, 2023
View reviewed changes
Copy link
Contributor

@srawlins srawlins left a comment

Choose a reason for hiding this comment

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

Wow, if I'm reading this right, no ignores were required except in samples, that's great news!

@chunhtai chunhtai self-requested a review July 5, 2023 22:48
@goderbauer goderbauer force-pushed the unreachable_from_main branch from 3c10976 to 4e40a87 Compare July 5, 2023 22:53
chunhtai
chunhtai approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM

dev/manual_tests/lib/actions.dart Outdated
_maxUndoLevels = value;
_pruneActions();
}
int get maxUndoLevels => _defaultMaxUndoLevels;
Copy link
Contributor

@chunhtai chunhtai Jul 5, 2023

Choose a reason for hiding this comment

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

hardcode this to 1000 or use _defaultMaxUndoLevels directly in code?

@goderbauer goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 5, 2023
@auto-submit auto-submit bot merged commit 55b6f04 into flutter:master Jul 6, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 6, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 6, 2023
@engine-flutter-autoroll
55b6f04 Enable unreachable_from_main lint - it is stable now!!1 (flut…
0e13e92 
…ter/flutter#129854)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 6, 2023
@engine-flutter-autoroll
Roll Flutter from 35085c3 to bc49cd1 (14 revisions) (#4387)
5727ee3 
flutter/flutter@35085c3...bc49cd1

2023-07-06 ian@hixie.ch Allow long-press gestures to continue even if buttons change. (flutter/flutter#127877)
2023-07-06 goderbauer@google.com Enable unreachable_from_main lint - it is stable now!!1 (flutter/flutter#129854)
2023-07-06 ian@hixie.ch Update labeler to new label names (flutter/flutter#130040)
2023-07-05 47639380+Snonky@users.noreply.github.com MergeableMaterial: Fix adding a slice and separating it (flutter/flutter#128804)
2023-07-05 ian@hixie.ch Update infrastructure issue template for new priority scheme (flutter/flutter#129741)
2023-07-05 737941+loic-sharma@users.noreply.github.com Fix typo in canvas example (flutter/flutter#129879)
2023-07-05 hans.muller@gmail.com Reland Fix AnimatedList & AnimatedGrid doesn't apply MediaQuery padding #129556 (flutter/flutter#129860)
2023-07-05 ian@hixie.ch Change from "created via performance template" to "from: performance template" (flutter/flutter#130035)
2023-07-05 109253501+pdblasi-google@users.noreply.github.com Removes deprecated APIs from v2.6 in `binding.dart` and `widget_tester.dart` (flutter/flutter#129663)
2023-07-05 helinx@google.com Add new hot reload case string (flutter/flutter#130008)
2023-07-05 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 987b621eac4e to bd2e42b203e1 (32 revisions) (flutter/flutter#130023)
2023-07-05 69246223+moylanm@users.noreply.github.com Add simple unit tests for annotations.dart file (flutter/flutter#128902)
2023-07-05 gipcjs@gmail.com fix a bug when android uses CupertinoPageTransitionsBuilder... (flutter/flutter#114303)
2023-07-05 piotr.fleury@gmail.com Add .env file support for  option `--dart-define-from-file` (flutter/flutter#128668)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC bmparr@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@HansMuller HansMuller mentioned this pull request Jul 7, 2023
Closed
@goderbauer goderbauer deleted the unreachable_from_main branch July 13, 2023 18:15
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
@engine-flutter-autoroll
55b6f04 Enable unreachable_from_main lint - it is stable now!!1 (flut…
4f55909 
…ter/flutter#129854)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
55b6f04 Enable unreachable_from_main lint - it is stable now!!1 (flut…
ecd631c 
…ter/flutter#129854)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
55b6f04 Enable unreachable_from_main lint - it is stable now!!1 (flut…
d9f30a8 
…ter/flutter#129854)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
55b6f04 Enable unreachable_from_main lint - it is stable now!!1 (flut…
05bf9cc 
…ter/flutter#129854)
@goderbauer goderbauer mentioned this pull request Jul 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srawlins srawlins srawlins approved these changes

@chunhtai chunhtai chunhtai approved these changes

@justinmc justinmc Awaiting requested review from justinmc

+1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@goderbauer @christopherfujino @srawlins @chunhtai @Hixie