dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
157 stars 44 forks source link

Help people generating ignores. Perhaps generate them by default. #370

Closed sigurdm closed 1 year ago

sigurdm commented 2 years ago

See https://github.com/dart-lang/pana/issues/1011

jonasfj commented 2 years ago

Specifically, most users of ffigen will probably want to inject comments like:

  // ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field

In the preamble...

Instead of doing it through the preamble, maybe add an option like ignore_dart_lints, example:

ffigen:
  output: 'generated_bindings.dart'
  headers:
    entry-points:
      - 'example.h'
  ignore_dart_lints: true # default value being true

where ignore_dart_lints defaults to true, and causes:

// ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field

to be inserted after the preamble.

That way users of ffigen won't have to manually add these ignore_for_file stuff in the preamble. In practice it's probably rare that users care about the naming convention of generated bindings. Most C libraries won't adhere to our styles anyways.

This is just to make it easier to get the right ignore_for_file comments, by doing the right thing by default.

bwilkerson commented 2 years ago

If it's of use, there is also support now for the following:

// ignore_for_file: type = lint

This turns off all lint reporting in the file. It is intended to be used by code generators to avoid having to write out the name of every lint rule that might be violated by the generated code.

Haidar0096 commented 1 year ago

If it's of use, there is also support now for the following:

// ignore_for_file: type = lint

This turns off all lint reporting in the file. It is intended to be used by code generators to avoid having to write out the name of every lint rule that might be violated by the generated code.

will this preamble prevent pub.dev's server analysis from deducting point from my package for unused members in the generated files by ffigen?

sigurdm commented 1 year ago

will this preamble prevent pub.dev's server analysis from deducting point from my package for unused members in the generated files by ffigen?

Yes it should, pub.dev's analysis respects // ignores.

Haidar0096 commented 1 year ago

@sigurdm It didn't work, here is the file (look at the preambles) and here is the package (deducted 10 points)

sigurdm commented 1 year ago

Seems like the rules unused_element, unused_field are not lints (but hints?).

I don't know if there is a blanket way to ignore all hints (@bwilkerson is there? For the end-user the distinction here seems unuseful).

Otherwise you should manually add those two to the preamble and things should work.

You can remove the exclude from the analysis_options file and test locally, there should be nothing reported by flutter analyze.

bwilkerson commented 1 year ago

Correct. Both unused_element and unused_field are warnings (hints), not lints.

The distinction between a warning and a lint is that warnings are enabled by default, while lints are disabled unless the user has explicitly enabled them.

The motivation for supporting // ignore_for_file: type = lint was for generated code. It isn't possible for a code generator to produce code that will conform to every lint (some are contradictory), and even if that were possible it seems like too high a bar. Code generator authors can't know ahead of time which lints are enabled, so we built in a mechanism by which they could disable all lints in the generated code.

The same motivation doesn't apply to warnings. Warnings can be known ahead of time and represent real problems in the code, many of which are guaranteed to produce runtime exceptions if executed. It was our assessment that code generators can and should be written in such a way as to prevent any code that would produce a warning. (If you think that assessment is wrong, then I'm happy to discuss why.) As a result, we don't support a blanket way of disabling all warnings.

But for cases like these, where the code is misleading and inefficient, but won't generate a runtime exception, it is still possible to ignore individual warnings in the generated code. That said, I would still encourage code generation authors to attempt to avoid needing to ignore any warnings because the generated code will be better for it.