google / pedantic Goto Github PK
View Code? Open in Web Editor NEWHow to get the most value from Dart static analysis
Home Page: https://pub.dev/packages/pedantic
License: BSD 3-Clause "New" or "Revised" License
How to get the most value from Dart static analysis
Home Page: https://pub.dev/packages/pedantic
License: BSD 3-Clause "New" or "Revised" License
I asked similar question on this comment, but the team didn't really respond quite well. I am thus creating a new issue.
You can either use as
or type annotation to upcast or downcast things, let's take this example, where I have a SizedBox
and I want to upcast to a Widget
, so that I don't want to allow users to use width
property on it, in this example I am using as
to upcast, but this gives me a warning:
(Whenever I am writing bad
in comment, that means it is undesired behavior, and good
for desired one)
const widget = SizedBox() as Widget; // warning
final width = widget.width; // error (good)
Now if I take warning into consideration and modify my code to silence pedantic, I can do:
const widget = SizedBox(); // no warning (good)
final width = widget.width; // no error (bad)
But this allows me to access width
which I don't want.
Second solution is to use type annotation like:
const Widget widget = SizedBox(); // no warning (good)
final width = widget.width; // error (good)
So you can clearly see if I use as
, I get myself in trouble and can only do limited things but if I use type annotation, I can do things the way I want. Let's take one more example into consideration (thanks to @eernstg )
final foo = 1 as String; // no error (bad)
final String bar = 1; // error (good)
As you can see if I am again comparing as
with type annotation, the latter has advantages almost everywhere.
Because new
is optional in Dart 2, does it make sense to discourage its usage? Also I guess that by extension, if unnecessary_new
were included, then that would imply that unnecessary_const
should also be included.
Do you think this that this is a worthwhile addition, or unnecessary?
I've seen CIs for several projects break after the release of the latest package:pedantic
- v1.6.0
.
In the readme for this project, I see:
This example uses a pinned version because every added lint rule could break a build for projects using it.
You might consider either
>=1.5.0 <1.6.0
).^1.0.0
and adjust to new versions of pedantic at a more leisurely pace. That would probably also follow the guidance of semver more closely.While unsafe_html
is an enabled lint at Google, it really only makes sense because of our build rules, access controls, concepts of "owners," etc.
I think users are getting more frustrated and confused by this lint rule, and the fact that you cannot // ignore:
violations. See dart-lang/sdk#45230 and dart-lang/linter#2348.
I think it should be removed from the list provided in pedantic's analysis options.
(Derived from dart-lang/sdk#35513, a request for `@sideEffects)
Take a class like so:
class Store {
Map _m;
Map get m {
// Fetch m from somewhere, store in `_m`, and return it.
}
}
There are cases where code (tests in particular), are testing the side effects without using the returned value. So you might see something like:
new S().m;
This looks like an unnecessary_statements violation, because unnecessary_statements assumes that all property access calls, even if they have side effects, are not just called for the side effects. Here are some other lint rules that assume that property access is not called for the side effects:
class A {
B get b {
print('b was gotten');
return B();
}
}
class B {
int c, d;
}
void f() {
if (a.b.c == null) {
a.b.c = 7; // triggers a prefer_conditional_assignment lint report.
}
a.b
..c = 1
..d = 2;
a.b.c = 3; // triggers a cascade_invocations lint report.
a.b; // triggers unnecessary_statement lint report.
a.b.c = 7;
return a.b.c; // triggers a join_return_with_assignment lint report.
}
It could be called externalSideEffects
or importantSideEffects
or meaningfulSideEffects
or something as well.
So the above code would be rewritten, to avoid the lint rules:
class A {
B get b {
print('b was gotten');
return B();
}
}
class B {
int c, d;
}
void f() {
if (externalSideEffects(a.b).c == null) {
a.b.c = 7; // no more prefer_conditional_assignment
}
externalSideEffects(a.b)
..c = 1
..d = 2;
a.b.c = 3; // no more cascade_invocations
externalSideEffects(a.b); // no more unnecessary_statement
a.b.c = 7;
return externalSideEffects(a.b).c; // no more join_return_with_assignment
}
when i run 'flutter format -n --set-exit-if-changed ." , it return 'Formatting failed: 1' , how can i ignore the rule of lines_longer_than_80_chars to let it reture success
The instructions in https://github.com/dart-lang/pedantic#enabled-lints suggest this dependency snippet:
dependencies:
pedantic: '1.1.0'
Should this be updated to 1.4.0
? What about unpinning and changing it to ^1.0.0
?
/cc @davidmorgan
Because every version of flutter_test from sdk depends on pedantic 1.8.0+1 and beyond depends on pedantic ^1.9.0, flutter_test from sdk is forbidden.
So, because beyond depends on flutter_test any from sdk, version solving failed.
pub get failed (1; So, because beyond depends on flutter_test any from sdk, version solving failed.
Is this intentional? We need to wait until the Flutter test SDK uses 1.9.0 before we can use it in our projects?
class Sample {
Sample(this.answer);
int answer;
factory Sample.fortyTwo() => Sample(42); // <- "Sort constructor declarations before other members"
}
After adding pedantic
to a project, I am seeing this warning:
warning • The include file package:pedantic/analysis_options.yaml in /Users/anthony/github/awhitford/Animated-Text-Kit/analysis_options.yaml
cannot be found • analysis_options.yaml:1:11 • include_file_not_found
The analysis_options.yaml
is as prescribed:
include: package:pedantic/analysis_options.yaml
Since I am only using pedantic
for lints, I added it to dev_dependencies
.
Note that the rules seem to be working and I am seeing results from flutter analyze
.
However, I noticed that this warning goes away if I move pedantic
to dependencies
. (But I don't really want to because it should not be necessary.) As a result, this warning looks invalid.
const ValueSlider(
{Key key,
@required this.onChanged,
@required this.labels,
@required this.values,
@required this.type,
this.status = AttendanceStatus.none})
: super(key: key);
this is caught by the linter even though Flutter puts Key first by default.
The latest version of the 'default' google analyser settings emit the warning:
Omit type annotations for local variables (omit_local_variables_types).
In many cases this improves the readability of code by removing redundant information.
e.g.
var i = 1;
The removal of the type (int) here doesn't affect the codes readability as the type of i
is obvious.
There are however many cases where removing the type actually makes the code harder to read and for a human to do static analysis.
e.g.
var code = _extractCode(monetaryAmount, codeLength);
There is no information on this line that tells a reader what type 'code' is.
The lack of type information forces the reader to stop and check what type _extractCode returns.
If you are analysing a class of some complexity, having to stop and check the type of numerous variables significantly slows down the process.
In a world of auto completion the reduction of a type to 'var' provides little reduction in typing.
The only other argument for reducing a type to 'var' is visual clutter.
Reduction in visual clutter should focus on removing redundant information, in the examples given the information being removed is not redundant.
I would suggest that the Omit type annotations warning should only be emitted when it is obvious what the type is.
In the following example, you can see the class members a
and b
are both initialized at the time of declaration. There's no benefit of marking them late
as it is redundant. Can we show a lint warning here?
class Foo {
late final int a = 0; // late is redundant
late final int b = getB(); // late is redundant
int getB() => 1;
}
http://dart-lang.github.io/linter/lints/file_names.html
New user just hit this here: https://pub.dartlang.org/packages/m3u/versions/0.0.2#-analysis-tab- (likely using Windows)
Would be good to enforce.
Now that a Dart team-recommended set of lint rules has been published, I think it makes sense to think of pedantic as the layer on top of those rules which we value highly internally.
(I know that that is not the actual situation; the pedantic rules are just completely separate, but in terms of analysis options files for users, I think it would be pragmatic to think of pedantic this way.)
Only one analysis options file can be include
d into another; you cannot include a list of analysis options files. Perhaps you can some day, but there is some hashing out w.r.t. merging that needs doing, and it might not be a high priority item. In the meantime, the only way to include multiple analysis options files into your own is by daisy chaining them. package:lints/recommended.yaml
includes package:lints/core.yaml
, so you get the lint rules from each.
It would be great for pedantic to provide a new analysis options file which includes package:lints/recommended.yaml
. Something like package:pedantic/recommended.yaml
. WDYT?
Note on Flutter SDK Style: some lints were created specifically to support Flutter SDK development. Flutter app developers should instead use standard Dart style as described in Effective Dart, and should not use these lints.
This makes it clear that the recommendation for app developers is to defer style decisions to pedantic
. Is this also the case for general-purpose and flutter-specific libraries?
Related to flutter/flutter#29228
Just one more thing...
Could you tag the v1.9.0 release commit so I can pull it into the dart SDK through our deps downloading mechanism?
I am getting noisy warning Unused import: 'dart:ui'.
for the file 'generated_plugin_registrant.dart'
dart:ui is really unused here, however I do not want to update an auto-generated file
I had to manually add await_only_futures
and cannot really think of a case where one would not like to do this, so perhaps add this rule by default?
➜ flutter pub get
Because pedantic >=1.9.1 depends on meta ^1.2.0 which doesn't match any versions, pedantic >=1.9.1 is forbidden.
So, because [xxx] depends on pedantic ^1.9.1, version solving failed.
Running "flutter pub get" in [xxx]...
pub get failed (1; So, because [xxx] depends on pedantic ^1.9.1, version solving failed.)
The most recent stable version of meta is 1.1.8.
Vague title, but I couldn't resist it sorry.
Here's my situation: I want to use pedantic but I've also got quite a few extra lints that I want enabled. Some of those will almost certainly be included in pedantic itself at some point. Since the point of pedantic is to adhere to Google's standard, the lints I add are useful as a point of difference. However, nothing guarantees that they are a point of difference.
What's the guidance in this situation? It would be nice to know that if my lint configuration becomes redundant at some point (due to a newer pedantic release) that I would be notified, but it seems that this is not the case (I just tried adding a lint that pedantic has already enabled). Is my only recourse to manually pick through the lints I have enabled and see whether they're redundant? Seems finicky.
...and not a dev dependency.
The readme says:
Note that everything here fits within the guidelines set out in Effective Dart. You could think of that document as the design and this package as one possible partial implementation.
I don't think that is quite true. Effective Dart is intentionally not opinionated about double vs single quotes, for instance.
I think we should find a way to explain that in addition to best practices, we also add lints for the sake of having consistent style when there are competing but equal options.
Since this rule is enabled by default in pedantic, my project has more than 6000 warnings.
Would it be an option to add a dartfmt fix for this, as there's no easy way to change it for us without analyzing each occurrence for an escaped single quote in the strings.
From modulovalue/extra_pedantic#1:
This is intended to help prevent the dangling else problem, however, to avoid that problem, curly braces have to be added in very few scenarios and in all the other cases this lint is unhelpful.
I do not understand why this lint will add warnings for statements like the following:
if (condition)
someLongStatementThatIsAutomaticallyFormattedOntoANewLine();
for (...) doStuff();
Having to add curly braces in these scenarios seems unproductive to me.
I created a linter issue about this as well: dart-lang/linter#1763
include: package:pedantic/analysis_options.yaml
linter:
rules:
- camel_case_types
This would add camel_case_types
rule.
Is there any way to ignore some rules?
This could be a list with comments – or a Map with key (lint) + value (description of why not)
It'd be nice to have this in a machine-readable format so it can be used in tools
I am using the Flutter INTL package in my app for internationalization. The package generates a generated
folder and I want the analyzer to ignore it.
I followed the Exclude code from analysis article however, it seems it doesn't work as two of the generated files won't be disappeared from the list.
This is my pubspec.yaml file.
name: luma_app
description: Luma My Account
# The following line prevents the package from being accidentally published to
# pub.dev using `pub publish`. This is preferred for private packages.
publish_to: 'none' # Remove this line if you wish to publish to pub.dev
# The following defines the version and build number for your application.
# A version number is three numbers separated by dots, like 1.2.43
# followed by an optional build number separated by a +.
# Both the version and the builder number may be overridden in flutter
# build by specifying --build-name and --build-number, respectively.
# In Android, build-name is used as versionName while build-number used as versionCode.
# Read more about Android versioning at https://developer.android.com/studio/publish/versioning
# In iOS, build-name is used as CFBundleShortVersionString while build-number used as CFBundleVersion.
# Read more about iOS versioning at
# https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html
version: 1.0.0+1
environment:
sdk: ">=2.7.0 <3.0.0"
dependencies:
flutter:
sdk: flutter
flutter_localizations:
sdk: flutter
# The following adds the Cupertino Icons font to your application.
# Use with the CupertinoIcons class for iOS style icons.
cupertino_icons: ^1.0.0
# Firebase related libraries
firebase_core: ^0.5.3 # https://pub.dev/packages/firebase_core
firebase_crashlytics: ^0.2.4 # https://pub.dev/packages/firebase_crashlytics
firebase_analytics: ^6.3.0 # https://pub.dev/packages/firebase_analytics
firebase_auth: ^0.18.4+1 # https://pub.dev/packages/firebase_auth
cloud_firestore: ^0.14.4 # https://pub.dev/packages/cloud_firestore
firebase_storage: ^5.2.0 # https://pub.dev/packages/firebase_storage
# Internationalization package helps us to manage different languages
intl: ^0.16.1 # https://pub.dev/packages/intl
provider: ^4.3.2+3 # https://pub.dev/packages/provider
google_fonts: ^1.1.1 # https://pub.dev/packages/google_fonts
package_info: ^0.4.3+2 # https://pub.dev/packages/package_info/install
shared_preferences: ^0.5.12+4 # https://pub.dev/packages/shared_preferences
get_it: ^5.0.3 # https://pub.dev/packages/get_it
flutter_svg: ^0.19.1 # https://pub.dev/packages/flutter_svg
native_pdf_view: ^3.9.0 # https://pub.dev/packages/native_pdf_view
dev_dependencies:
flutter_test:
sdk: flutter
pedantic: ^1.9.2 # https://pub.dev/packages/pedantic
# effective_dart: ^1.3.0 # https://pub.dev/packages/effective_dart
# For information on the generic Dart part of this file, see the
# following page: https://dart.dev/tools/pub/pubspec
# The following section is specific to Flutter.
flutter:
generate: true # This is for localizations. Don't remove it.
# The following line ensures that the Material Icons font is
# included with your application, so that you can use the icons in
# the material Icons class.
uses-material-design: true
assets:
- assets/google_fonts/
- assets/images/
flutter_intl:
enabled: true
The unawaited check doesn't flag calls to async methods which return plain void rather than Future<void>, ie:
void main() async {
try {
doThing(); // No unawaited error here
} catch (e) {
print("caught $e");
}
}
// Returns void instead of Future<void>
void doThing() async {
await Future.delayed(Duration(seconds: 1));
throw Exception("oops");
}
I'm not really clear on the semantics of returning void instead of Future from doThing(). IIRC the compiler used to always require returning a Future from async methods and at some point changed to allow just void, but that still seems to function as an awaitable future.
In the example above, awaiting doThing() results in the exception being caught while not awaiting does not.
Seems like this should be flagged?
I was reading through Unused Lints when I saw "... does not reflect common usage".
What does that really mean? Is it actually a deterrent for using a rule? Are the rules beneficial would it not be uncommon practice?
future.unawaited
vs. unawaited(future)
is a matter of taste :) . Both could silent unawaited_futures
. What do you think of adding the following extension?
extension FutureExt<T> on Future<T> {
void get unawaited {}
}
Would it be possible to have a detection of hardcoded strings through the project. Once we have finished our project and want to internationalize it, it is painful to search for strings contained in " and ' through dozens of file.
I might be wrong, but I didn't find a proper solution to find them all easily.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.