[BUG][DART] Fields named json get shadowed by ToJson and become unserializable due to cyclic pointer reference

[0xNF]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Here's a repo for the issue: https://github.com/0xNF/openapi_gen_dart_json_shadow

The Dart SDK generator will incorrectly shadow field variables named json in the generated toJson() method, resulting in a cyclic pointer loop when running the toJson() method.

openapi-generator version

jar name: openapi-generator-cli-6.0.0-20220412.074015-127.jar
jar version: 6.0.0-SNAPSHOT

OpenAPI declaration file content or url

openapi: 3.0.3
info:
  version: "1.0"
  title: Dart Shadow Json Demo
servers:
  - url: 'localhost'
    variables:
      host:
        default: localhost
components:
  schemas:
    ItemWithFieldNamedJson:
      type: object
      properties:
        json:
          type: object
          additionalProperties: {}
paths:
  /:
    get:
      operationId: shadow
      responses:
        '200':
          description: produces a shadowed json field when generated in dart
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ItemWithFieldNamedJson'

Generation Details

See: https://github.com/0xNF/openapi_gen_dart_json_shadow/blob/master/shadowed/lib/model/item_with_field_named_json.dart

class ItemWithFieldNamedJson {
  ItemWithFieldNamedJson({
    this.json = const {},
  });

  Map<String, Object> json;

  Map<String, dynamic> toJson() {
    final json = <String, dynamic>{};
      json[r'json'] = json;
    return json;
  }

Steps to reproduce

1. Run the generator command

• java -jar openapi-generator-cli-6.0.0-20220412.074015-127.jar generate -i .\shadowspec.yaml -g dart -o shadowed

2. Examine the shadowed/lib/model/item_with_field_named_json.dart
3. See the shadowed variable on line 34, and the infinite reference on line 35

Related issues/PRs

Use another parameter name to stop variable shadowing. related to #10263

Suggest a fix

The hard fix:

• For any variables that are generated by the Generator, use static analysis to determine if there are any conflicting variable names in the namespace in the generated functions, and then modify the desired generated variable name appropriately, repeating as many times as necessary to ensure truly unique variable names within the functions scope.

The easier fix:

• The Dart generator already knows about reserved keywords. For fields that are named int, generated fields are named int_ with an underscore. Extend this idea to json. This is a half measure though, as if I name my field variable json_, then we're back to being shadowed.

[kuhnroyal]

Yea probably can go with _json inside the method. Feel free to create a PR, this should be pretty easy.

Or something like:

  Map<String, dynamic> toJson() {
    return <String, dynamic>{
	  r'json': json,
	};
  }

CC @agilob

[0xNF]

@kuhnroyal
I found another instance of the same bug -- this time with variables named path where the calls to Future<Response> xWithHttpInfo({ String? path }); have the path variable shadowed by the generated path string:

  Future<Response> getFolderWithHttpInfo({ String? path, }) async {
    final path = r'/some_random_path';
    [...]
}

I can submit a fix for this newly found path bug, but it would probably be better if we could agree to a bigger fix -- for instance, renaming every generated variable to have a leading underscore if its scope comes in contact with any open-api specified variable names.

As an example, in the same withHttpInfo method, I see the following generated variables:

    Object? postBody;

    final queryParams = <QueryParam>[];
    final headerParams = <String, String>{};
    final formParams = <String, String>{};


    const authNames = <String>[];
    const contentTypes = <String>[];

Having these precludes any openapi variables from being defined with these names lest they get shadowed.
Admittedly, adding an underscore to these variables is just kicking the can down the road, but I guess having a leading underscore as a public variable name is quite a bit less likely than not.

[kuhnroyal]

A lot of these variables can be avoided by nesting, spread operators etc. - that's what I did in the Dio generator. Automatically renaming them is not really feasible, would have to keep additional reserved word lists which will ultimately be out of sync. Not sure what other generators do in these cases.

[0xNF]

Is it acceptable if I rewrite most of these generated methods to use spreads, etc to avoid these naming conflicts?

I don't know how big of a commit the repo maintainers (e.g, you) are willing to accept at once.

[kuhnroyal]

It is ok to rewrite. We can review, it would be great if we could add some tests. It is also good to get this in before 6.0.0 release.

[0xNF]

What's the timeline for the 6.0 release?

[kuhnroyal]

What's the timeline for the 6.0 release?

~3 weeks