Skip to content

Linter tool support for embedded SDKs. #57367

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

Closed
yyoon opened this issue Aug 25, 2016 · 13 comments
Closed

Linter tool support for embedded SDKs. #57367

yyoon opened this issue Aug 25, 2016 · 13 comments
Labels
customer-flutter devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. type-enhancement A request for a change that isn't a bug

Comments

@yyoon
Copy link
Contributor

yyoon commented Aug 25, 2016

In our Flutter code, we use some classes / enums in dart:ui extended by sky_engine.
Linter seems to think these classes are undefined, which is not the case.

Steps to reproduce:

  • Create a new flutter project using flutter create
  • In the created lib/main.dart, Modify the text widget in the sample code to use FontWeight.bold style, which is defined in dart:ui.
child: new Text(
  'Button tapped $_counter time${ _counter == 1 ? '' : 's' }.',
  style: new TextStyle(fontWeight: FontWeight.bold),
),
  • This code runs fine and dartanalyzer doesn't show any errors, but running pub global linter . gives the following warning:
<omitted>/foo/lib/main.dart 33:46 [static warning] Undefined name 'FontWeight'
            style: new TextStyle(fontWeight: FontWeight.bold),
                                             ^^^^^^^^^^
<omitted>/foo/lib/main.dart 10:7 [lint] Document all public members
class FlutterDemo extends StatefulWidget {
      ^^^^^^^^^^^
<omitted>/foo/lib/main.dart 11:3 [lint] Document all public members
  FlutterDemo({Key key}) : super(key: key);
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

2 files analyzed, 3 issues found, in 5220 ms.

The last two are valid lint issues, but the first one is false positive.

@dvdwasibi @sethladd

@bwilkerson
Copy link
Member

Perhaps the linter hasn't been updated to understand embedder files. Note that dartanalyzer will also produce lints when passed the --lints flag, so that might be a work-around until we can fix the problem.

@yyoon
Copy link
Contributor Author

yyoon commented Aug 25, 2016

Does the --lints flag do the exact same thing as linter? I tried it just now and it doesn't seem to give me the "Document all public memebers" issue.

@bwilkerson
Copy link
Member

It is suppose to. I'm not sure why they're different.

Does dartanalyzer report other lints enabled in the analysis options file?

@yyoon
Copy link
Contributor Author

yyoon commented Aug 25, 2016

I see. We didn't have the .analysis_options file in the project. I'll add it and use dartanalyzer for now.

@pq
Copy link
Member

pq commented Aug 29, 2016

Perhaps the linter hasn't been updated to understand embedder files.

👍 This is right.

@yyoon : is dartanalyzer working for you? Alternatively, flutter analyze should also do the trick?

@yyoon
Copy link
Contributor Author

yyoon commented Aug 29, 2016

@pq dartanalyzer is working fine for now, but it doesn't display the issues as nice as linter does. We would probably want to go back to linter when this issue is fixed.

BTW, why are there these two separate tools in the first place, when dartanalyzer can just do the trick? Are they expected to be used in different situations?

@sethladd
Copy link
Contributor

cc @kevmoo

@pq
Copy link
Member

pq commented Aug 30, 2016

BTW, why are there these two separate tools in the first place, when dartanalyzer can just do the trick? Are they expected to be used in different situations?

The linter command line was created when we were bootstrapping the linter project and did not have a plugin story for the analyzer yet. It's also quite handy for testing lints. The preferred way, moving forward, would be to use dartanalyzer. For convenience, I'd like to update the linter binary as well to essentially just forward to the analyzer.

Having said that, I did add some display tweaks that you may miss. What specifically would you like to see continue? Maybe we can upstream those bits to dartanalyzer...

@yyoon
Copy link
Contributor Author

yyoon commented Aug 31, 2016

The linter shows the actual line of code and highlights the exact part of the line that the issue is referring to:

foo/lib/main.dart 10:7 [lint] Document all public members
class FlutterDemo extends StatefulWidget {
      ^^^^^^^^^^^

whereas dartanalyzer --lints only shows the file name and the line number:

[lint] Document all public members (/Users/youngseokyoon/Programming/baku/modules/email/lib/main.dart, line 20, col 7)

It'd be nice to have the same display format in dartanalyzer.

@pq
Copy link
Member

pq commented Aug 31, 2016

Thanks! Feel free to open a feature request on the analyzer.

cc @bwilkerson

@yyoon
Copy link
Contributor Author

yyoon commented Aug 31, 2016

Just created on issue there. Thanks!

@pq pq changed the title False positives of undefined name / undefined class in Flutter code Linter tool support for embedded libraries. Sep 1, 2016
@pq pq added the type-enhancement A request for a change that isn't a bug label Sep 1, 2016
@pq
Copy link
Member

pq commented Sep 1, 2016

Title updated to highlight the underlying issue (the linter tool does not know about embedded SDKs).

@pq pq changed the title Linter tool support for embedded libraries. Linter tool support for embedded SDKs. Sep 1, 2016
@pq
Copy link
Member

pq commented Jun 21, 2018

Closing in favor of any follow-ups on the analyzer side.

@pq pq closed this as completed Jun 21, 2018
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-flutter devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants