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

Analyzer doesn't detect changes in l10n Flutter tool. #3074

Closed
jamesblasco opened this issue Jan 16, 2021 · 9 comments
Closed

Analyzer doesn't detect changes in l10n Flutter tool. #3074

jamesblasco opened this issue Jan 16, 2021 · 9 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug
Milestone

Comments

@jamesblasco
Copy link

Describe the bug
https://flutter.dev/docs/development/accessibility-and-localization/internationalization

In Flutter 1.22, a new tool for generating translations was released. This tool generates a synthetic package in the root
.dart_tool/flutter_gen/l10n_gen and updates the code when flutter run is running and the arb template file changes .

But the new code generated inside that package is not reanalyzed and saw an error like if the methods inside the package don't exists. Once the file .dart_tool/flutter_gen/l10n_gen/app_localizations.dart is opened the error disapears.

To Reproduce
Steps to reproduce the behavior:

  1. Follow the steps at https://flutter.dev/docs/development/accessibility-and-localization/internationalization

Screenshots
If applicable, add screenshots to help explain your problem.

Versions (please complete the following information):

  • VS Code version: 1.52.1
  • Dart extension version: 3.18.1
  • Dart/Flutter SDK version: Channel dev, 1.26.0-8.0.pre, on macOS 11.1 20C69 darwin-x64, locale es-E
@DanTup
Copy link
Member

DanTup commented Jan 18, 2021

@bwilkerson I can reproduce this in a test - it appears that files inside .dart_tool are analyzed at context creation but if they are modified while the analysis server is active, the updates are ignored.

It seems to be caused by this line in the watcher handler that skips anything in dot-folders:

https://github.com/dart-lang/sdk/blob/04e8aa58ee81bed52a63d4bbb10791a9eb018d7e/pkg/analysis_server/lib/src/context_manager.dart#L1319

It looks like that was added in dart-lang/sdk@f9aebdd to fix dart-lang/sdk#24212 - although perhaps that would also now also fixed by dart-lang/sdk@0450462 (which prevents the errors being sent to the user, but doesn't prevent analysis).

@bwilkerson
Copy link

I suspect that you're right, that if we re-analyzed files in hidden folders we would now be OK. I certainly think it's worth doing an experiment.

@DanTup
Copy link
Member

DanTup commented Jan 19, 2021

@bwilkerson with the change at https://dart-review.googlesource.com/c/sdk/+/179776/ this seems to work as expected. I've opened a Flutter project, fetched packages, built and run and I haven't noticed any negative effects. There is likely some overhead in that files that were previously just ignored will be analyzed (though this seems unavoidable to support something like the flutter_gen project that lives inside .dart_tool).

It does however break test_watch_addFile_pathContainsDotFile. I think this is probably expected but would like a second opinion. The test is checking that if you explicitly add (eg. open in the ide) a file inside a dot file that it's also not included in the context, although with this change it will be, it just would not produce error notifications. If that seems reasonable to you, I can remove it, or invert its logic (in which case it would function much like the test immediately below it).

@bwilkerson
Copy link

If analyzing all changed files in those directories proves to be a problem, we can probably find a way to optimize it further. That said, I'm going to ask @scheglov for his opinion on this one.

@scheglov
Copy link

It seems reasonable to me that requirements changed, and now we want to handle some files in dot-directories. I think that the biggest reason to avoid doing this was to avoid handling changes in not user facing directories like .git. So, I would limit it to specifically .dart_tool and package:build.

@DanTup
Copy link
Member

DanTup commented Jan 25, 2021

I've updated https://dart-review.googlesource.com/c/sdk/+/179776 to only handle .dart_tool (I'm not aware of any dot-folders package:build writes to outside of that, but more can be added if that's not true).

@DanTup DanTup added upstream in dart / flutter Needs changing in Dart or Flutter in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server and removed upstream in dart / flutter Needs changing in Dart or Flutter labels Jan 26, 2021
@DanTup DanTup added this to the v3.20.0 milestone Jan 26, 2021
dart-bot pushed a commit to dart-lang/sdk that referenced this issue Jan 27, 2021
These files were already analyzed at startup but not when modified. This meant code generated by flutter_gen would not update unless the analysis server was restarted/the project was reanalyzed.

Fixes Dart-Code/Dart-Code#3074.

Change-Id: If0c88c1e5ab7e796046e7e776a42c3d904944405
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/179776
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
@DanTup
Copy link
Member

DanTup commented Feb 1, 2021

This has been fixed in dart-lang/sdk@77353dc which should be included in a future Dart/Flutter SDK release.

@DanTup DanTup closed this as completed Feb 1, 2021
@orestesgaolin
Copy link

I wonder if you know which Flutter or DartCode version is going to include this fix?

@DanTup
Copy link
Member

DanTup commented Mar 14, 2022

@orestesgaolin the fix above should have shipped in stable releases quite some time ago, but perhaps you're still seeing #3438? The fix for that was in the Dart SDK, but looks like it wasn't in v2.16 (the current stable), so I'd expect it to be in the next major Dart/Flutter release.

I think it's probably already in the Flutter beta channel, as that reports 2.17.0-69.2.beta for the Dart SDK and the commit above includes the tag 2.17.0-20.0.dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug
Projects
None yet
Development

No branches or pull requests

5 participants