google / pedantic Goto Github PK

Screenshot:

It doesn't show any warning in first line, why is that so?

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?

...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.

(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
}

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 is effectively a revert of #57.

See dart-lang/sdk#42635 for context.

cc @davidmorgan

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

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

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.

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'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

• making this more explicit - only recommend that consumers use specific versions, or pin to narrow ranges (>=1.5.0 <1.6.0).
• or, only make pedantic more restrictive - adding more rules - on upgrades to major versions. That would allow projects to use deps like ^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.

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

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.

➜ 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.

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?

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.

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

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

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;
}

class Sample {
Sample(this.answer);
int answer;
factory Sample.fortyTwo() => Sample(42); // <- "Sort constructor declarations before other members"
}

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.

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.

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?

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?

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?

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 {}
}

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

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

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.

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 included 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?

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.

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.