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.

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

Add support to dart analyze for analyzing the Dart SDK libraries #48959

Closed
srawlins opened this issue May 4, 2022 · 6 comments
Closed

Add support to dart analyze for analyzing the Dart SDK libraries #48959

srawlins opened this issue May 4, 2022 · 6 comments
Assignees
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-analyze Issues related to the 'dart analyze' tool P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

srawlins commented May 4, 2022

As needed for #48457

This is currently supported via dartanalyzer --dart-sdk=/Users/foo/src/dart-repo/sdk/sdk for example.

@srawlins srawlins added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-analyze Issues related to the 'dart analyze' tool labels May 4, 2022
@srawlins srawlins mentioned this issue May 4, 2022
5 tasks
@srawlins srawlins self-assigned this May 5, 2022
copybara-service bot pushed a commit that referenced this issue May 6, 2022
Bug: #48959
Change-Id: I4c386f1f7c474c1ff809d15d597d3f8f2de0418a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243847
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
@srawlins srawlins closed this as completed May 6, 2022
@whesse
Copy link
Contributor

whesse commented May 6, 2022

The new test that is added fails because it does not match the wording of the error message.

It was not found on presubmit because the test pkg/dartdev/test/commands/analyze_test is flaky on all platforms except the new arm64 macos platform. This also points out that a flaky test is not useful any more to find new failures.

@whesse
Copy link
Contributor

whesse commented May 16, 2022

I just was gardening the results feed again, and hit this issue, and tracked it down again, finding this issue I'd already filed. This is because the test was marked flaky on mac, and so when the flakiness expired after 100 failing runs, it showed up on the results feed. I went to the history, and didn't see the existing issue, because I didn't delete the configuration from the filtered search, which would have searched just on the name and found this issue. Instead, I debugged it again, and found the failing commit, and then found the comment on that commit.

@whesse whesse reopened this May 16, 2022
@bwilkerson
Copy link
Member

Would it be possible, given the path to a file or directory, to automatically detect that the path is inside of a checkout of the sdk repository and to not require the user to specify the path to a parent directory of the path being analyzed?

As far as I know, analyzing files inside a a checkout of the sdk repository is the only use case we have for the --dart-sdk flag, and if that's true it would be better to not add a flag that could be abused if we don't need to.

@srawlins
Copy link
Member Author

Would it be possible, given the path to a file or directory, to automatically detect that the path is inside of a checkout of the sdk repository and to not require the user to specify the path to a parent directory of the path being analyzed?

Yes I think so.

As far as I know, analyzing files inside a a checkout of the sdk repository is the only use case we have for the --dart-sdk flag, and if that's true it would be better to not add a flag that could be abused if we don't need to.

+1

I think this should be filed as a separate request, since the support for the flag has landed.

@scheglov
Copy link
Contributor

So, if we can identify the SDK repository, we can remove --dart-sdk option?

@srawlins
Copy link
Member Author

I would guess that is true. I don't really know the use case for the SDK test runner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-analyze Issues related to the 'dart analyze' tool P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants