Skip to content
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

generated code fails static analysis strong type checking #247

Closed
diablodale opened this issue Jun 22, 2018 · 10 comments
Closed

generated code fails static analysis strong type checking #247

diablodale opened this issue Jun 22, 2018 · 10 comments

Comments

@diablodale
Copy link

The code generated does not have types on several parameters. It also implicitly casts to a more specific type. This is loosely typed and fails static analysis w/ strong checking.

The generated code functions. Rather, these errors appear in IDEs and other tools that digest code analysis. So the fix is to provide a way to understand or rid the errors.

Setup

  • json_serializable 0.5.8
  • flutter 0.5.1
  • analysis_options.yaml with the following:
analyzer:
  strong-mode:
    implicit-casts: false
    implicit-dynamic: false

Repo steps

  1. Write a flutter app
  2. Create object Item which has some simple fields like a String and int.
  3. Create object MyGroup which has a field List<Item> myItems; and Set<Item> myItemSet;
  4. Annotate these objects and use common json_serializable procedure for them.
  5. flutter packages pub run build_runner build
  6. View the code in your IDE.

Result

Errors in your IDE for that generated code. You can see in the dart files and the missing param types.
In the below generated code, the e has no type declared. Then, the Iterable created from the Json is being assigned to a Set.

MyGroup _$MyGroupFromJson(Map<String, dynamic> json) {
  return new MyGroup(
      myItems: (json['myItems'] as List)
          ?.map((e) => e == null
              ? null
              : new Item.fromJson(e as Map<String, dynamic>))
          ?.toList(),
      myItemSet: (json['myItemSet'] as List)?.map((e) =>
          e == null ? null : new Item.fromJson(e as Map<String, dynamic>)),
  );
}

Possible Quick Fixes

  • Setup analysis_options.yaml to ignore the generated files with a glob that matches only the generated files with loose types.

or

  • Put dynamic before the two scenarios having the e parameter.
  • Put the type after the .map, for example ?.map<Item>((
  • Put new Set<Item>.from(...) around the whole Iterable
@kevmoo
Copy link
Collaborator

kevmoo commented Jun 26, 2018

I support implicit-casts: false, but not implicit-dynamic: false – the generated code just gets too complex.

Why are you using implicit-dynamic: false ?

@kevmoo kevmoo closed this as completed Jun 26, 2018
@diablodale
Copy link
Author

diablodale commented Jun 27, 2018

Why do I use implicit-dynamic: false? It's a process, experiential, perspective.

I've decades in strongly-checked languages and have learned that errant code is more easily found when types are declared, known, and checked. The rigidity forces programmers to think about the object/class/hierarchies, to fully declare their interfaces which helps both caller and implementer know the rules/boundaries, and to enable tools (like the analyzer) to verify that code is following the rules which are stated.

The user-programmer is responsible for declaring what they want the system to do. The compiler-implementer is responsible for turning that into machine code and optimizing. I rely on and trust the compiler-phase to remove/reduce overhead related to type-checking. And in some cases, to actually run faster/be small because it doesn't need to do run-time type checking so it can remove that chunk of code.

I think the use of generics/templates is a nice middle ground allowing strong typing while also allowing flexibility in writing generic-behaving code.

That topic set aside, this could also be a conversation on the topic of "When code is generated, does the Dart ecosystem/community want to treat it differently regarding analysis, checks, linting, etc. One might say that generated code coming from core teams should be reference-quality and pass all tests/lints/analyzers. That any coder can look at the generated code to learn language usage, techniques, etc.

Another approach on that generated code topic might be that it should not be seen by user-programmers, should be obfuscated, should not be leveraged as user-code techniques, etc. In this perspective, I recommend removing all unneeded whitespace ala minify and somehow prevent type-checking by static/analysis tools so that user-programmers are not distracted by those same tools when they report errant behavior/style in the generated code.

@kevmoo
Copy link
Collaborator

kevmoo commented Jun 28, 2018

See the discussion here #73

We are pondering some more strict checks that are less onerous than implicit-dynamic: false. We may consider updates once those checks are available.

@hpoul
Copy link

hpoul commented May 13, 2019

fwiw, i have made a very simple builder which allows to add // ignore_for_file: strong_mode_implicit_dynamic_parameter to the generated part files, which silences those errors: https://github.com/hpoul/builder_static_text has anyone found a better/simpler workaround? (since for me analyser exclude property did not work)

@kevmoo
Copy link
Collaborator

kevmoo commented May 13, 2019

fwiw, i have made a very simple builder which allows to add // ignore_for_file: strong_mode_implicit_dynamic_parameter to the generated part files, which silences those errors: https://github.com/hpoul/builder_static_text has anyone found a better/simpler workaround? (since for me analyser exclude property did not work)

Related to dart-lang/source_gen#370

We need to get this back!

@bsutton
Copy link

bsutton commented Oct 29, 2019

Can I add my voice to getting a resolution for this.
We use strong mode on all of projects as it reduces runtime errors.

analyzer:
 strong-mode:
    implicit-casts: false
    implicit-dynamic: false

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Sep 5, 2021

+1 We also face this problem. Strong mode is quite helpful as it reduces bugs!

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Sep 5, 2021

In addition, I find the workaround of strong_mode_implicit_dynamic_parameter does not work...

For example: After adding it, it still errors!

Is there any updates?

image

@kevmoo
Copy link
Collaborator

kevmoo commented Sep 5, 2021

We're likely not going to change this. It adds A LOT of extra code.

Try this in your build.yaml file.

# Read about `build.yaml` at https://pub.dev/packages/build_config
# To update generated code, run `pub run build_runner build`
targets:
  $default:
    builders:
      source_gen|combining_builder:
        options:
          ignore_for_file:
          - strong_mode_implicit_dynamic_parameter

...or similar for whichever lint/hint you want to ignore

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Sep 5, 2021

@kevmoo I see. Indeed it is renamed to implicit_dynamic_parameter now #557 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants