a14n / dart-google-maps

A library to use Google Maps JavaScript API v3 from Dart scripts.
Apache License 2.0
130 stars 66 forks source link

Many unused_field hints on private enums #73

Closed srawlins closed 4 years ago

srawlins commented 4 years ago

Recent versions of the dartanalyzer complain about unused static fields of private enums. This is hitting dart-google-maps hard:

lib/src/library/weather/wind_speed_unit.dart:

@jsEnum
@JsName('google.maps.weather.WindSpeedUnit')
enum _WindSpeedUnit { KILOMETERS_PER_HOUR, METERS_PER_SECOND, MILES_PER_HOUR }

It is possible that the analyzer should account for private classes/enums with a @JsName annotations (or maybe just @jsEnum?), but I'm not sure. @a14n would you mind sharing your thoughts? For example, why are these enums private? Why are they unused except for the curious code in lib/src/google_maps_src_weather.g.dart:

  // dumb code to remove analyzer hint for unused _WindSpeedUnit                                                                                    
  _WindSpeedUnit _dumbMethod1() => _dumbMethod2();                                                                                                  
  _WindSpeedUnit _dumbMethod2() => _dumbMethod1();

Should we just add more dummy code like the following:

  List<_WindSpeedUnit> _dumbMethod3() => _WindSpeedUnit.values;

(This would mark the enum values as used)

a14n commented 4 years ago

This package uses codegen. Those private enums was used as template for the generator (that is not working anymore). The workaround with dumb* function was here to avoid the generator's users to have to handle an analyzer warning on every enum templates (by adding an // ignore: unused_element).

For now we can simply add an ignore for the new warning. Could you give me its code?

Thanks for the feedback.

srawlins commented 4 years ago

The code is unused_field so // ignore: unused_field should do the trick.

Are you saying those files aren't used any more? Or they are used, but not for codegen? If the analyzer should not complain about the enum values, for example because they are marked @jsEnum, I'd love to fix that bug instead.

a14n commented 4 years ago

I just added the code to ignore comment in 9779ea1.

If the analyzer should not complain about the enum values, for example because they are marked @jsEnum, I'd love to fix that bug instead.

Nothing should be done at the analyzer level IMHO.

To explain a little more how it works. I use a code generator that takes unused enums (manually written as template) like:

@jsEnum
@JsName('google.maps.DistanceMatrixStatus')
// ignore: unused_element, unused_field
enum _DistanceMatrixStatus {
  INVALID_REQUEST,
  MAX_DIMENSIONS_EXCEEDED,
  MAX_ELEMENTS_EXCEEDED,
  OK,
  OVER_QUERY_LIMIT,
  REQUEST_DENIED,
  UNKNOWN_ERROR
}

to generate:


class DistanceMatrixStatus extends JsEnum {
  static final values = <DistanceMatrixStatus>[
    INVALID_REQUEST,
    MAX_DIMENSIONS_EXCEEDED,
    MAX_ELEMENTS_EXCEEDED,
    OK,
    OVER_QUERY_LIMIT,
    REQUEST_DENIED,
    UNKNOWN_ERROR
  ];
  static final INVALID_REQUEST = DistanceMatrixStatus._('INVALID_REQUEST',
      context['google']['maps']['DistanceMatrixStatus']['INVALID_REQUEST']);
  static final MAX_DIMENSIONS_EXCEEDED = DistanceMatrixStatus._(
      'MAX_DIMENSIONS_EXCEEDED',
      context['google']['maps']['DistanceMatrixStatus']
          ['MAX_DIMENSIONS_EXCEEDED']);
  static final MAX_ELEMENTS_EXCEEDED = DistanceMatrixStatus._(
      'MAX_ELEMENTS_EXCEEDED',
      context['google']['maps']['DistanceMatrixStatus']
          ['MAX_ELEMENTS_EXCEEDED']);
  static final OK = DistanceMatrixStatus._(
      'OK', context['google']['maps']['DistanceMatrixStatus']['OK']);
  static final OVER_QUERY_LIMIT = DistanceMatrixStatus._('OVER_QUERY_LIMIT',
      context['google']['maps']['DistanceMatrixStatus']['OVER_QUERY_LIMIT']);
  static final REQUEST_DENIED = DistanceMatrixStatus._('REQUEST_DENIED',
      context['google']['maps']['DistanceMatrixStatus']['REQUEST_DENIED']);
  static final UNKNOWN_ERROR = DistanceMatrixStatus._('UNKNOWN_ERROR',
      context['google']['maps']['DistanceMatrixStatus']['UNKNOWN_ERROR']);
  final String _name;
  DistanceMatrixStatus._(this._name, o) : super.created(o);

  @override
  String toString() => 'DistanceMatrixStatus.$_name';
}
srawlins commented 4 years ago

Ah, I see. Would you say that those files should be excluded from static analysis? Or is it still useful to get other analysis reports on these files?

I think your change makes sense. Thanks so much!

a14n commented 4 years ago

Would you say that those files should be excluded from static analysis?

No, other diagnostics can be useful.