lejard-h / chopper

Chopper is an http client generator using source_gen and inspired from Retrofit.
https://hadrien-lejard.gitbook.io/chopper
Other
714 stars 123 forks source link

Wrong generated code with classes of the same name #632

Open alvi-se opened 3 months ago

alvi-se commented 3 months ago

Steps to Reproduce

  1. Create a class that has the same name as one in the library. In my case, I have the Post class.
    
    import 'package:json_annotation/json_annotation.dart';

part 'post.g.dart';

@JsonSerializable() class Post { final String id; final String userId; final DateTime timestamp; final String content;

Post({ required this.id, required this.userId, required this.timestamp, required this.content });

factory Post.fromJson(Map<String, dynamic> json) => _$PostFromJson(json); }

2. Use it in a Chopper API service. Since both the model and the library have the `Post` class, I put the prefix `post_model` for the model.
```dart
import 'package:chopper/chopper.dart';
import 'package:example/models/post.dart' as post_model;

part 'post_api.chopper.dart';

@ChopperApi(baseUrl: '/posts')
abstract class PostApi extends ChopperService {

  static PostApi create() {
    final client = ChopperClient(
        baseUrl: Uri.parse(const String.fromEnvironment('API_URI')),
        services: [_$PostApi()],
        converter: const JsonConverter()
    );

    return _$PostApi(client);
  }

  @Get()
  Future<List<post_model.Post>> getPosts();
}
  1. Generate the code with the command dart run build_runner build --delete-conflicting-outputs. The generated code will be the following:
    
    // GENERATED CODE - DO NOT MODIFY BY HAND

part of 'post_api.dart';

// ** // ChopperGenerator // **

// coverage:ignore-file // ignore_forfile: type=lint final class $PostApi extends PostApi { _$PostApi([ChopperClient? client]) { if (client == null) return; this.client = client; }

@override final Type definitionType = PostApi;

@override Future<List> getPosts() async { final Uri $url = Uri.parse('/posts'); final Request $request = Request( 'GET', $url, client.baseUrl, ); final Response $response = await client.send<List, Post>($request); return $response.bodyOrThrow; } }

The code above doesn't compile. This happens because the generator uses the `Post` class without the prefix, which is the one in the Chopper library, used for POST requests.

Putting a prefix for the `chopper.dart` package gives the same result: the generated code doesn't put the prefix before Chopper classes and annotations, so the compiler doesn't find them.

The only workaround I found so far is to hide the `Post` class from Chopper, but this is not always possible: for example, if you need a POST request, you have to use both classes.

**Expected results:** generated code with prefixes, like the following:
```dart
// GENERATED CODE - DO NOT MODIFY BY HAND

part of 'post_api.dart';

// **************************************************************************
// ChopperGenerator
// **************************************************************************

// coverage:ignore-file
// ignore_for_file: type=lint
final class _$PostApi extends PostApi {
  _$PostApi([ChopperClient? client]) {
    if (client == null) return;
    this.client = client;
  }

  @override
  final Type definitionType = PostApi;

  @override
  Future<List<post_model.Post>> getPosts() async {
    final Uri $url = Uri.parse('/posts');
    final Request $request = Request(
      'GET',
      $url,
      client.baseUrl,
    );
    final Response $response = await client.send<List<post_model.Post>, post_model.Post>($request);
    return $response.bodyOrThrow;
  }
}

Logs ``` lib/models/post_api.chopper.dart:21:22: Error: The return type of the method '_$PostApi.getPosts' is 'Future>', which does not match the return type, 'Future>', of the overridden method, 'PostApi.getPosts'. - 'Future' is from 'dart:async'. - 'List' is from 'dart:core'. - 'Post/*1*/' is from 'package:chopper/src/annotations.dart' ('/C:/Users/alvif/AppData/Local/Pub/Cache/hosted/pub.dev/chopper-8.0.1+1/lib/src/annotations.dart'). - 'Post/*2*/' is from 'package:example/models/post.dart' ('lib/models/post.dart'). Change to a subtype of 'Future>'. Future> getPosts() async { ^ lib/models/post_api.dart:20:33: Context: This is the overridden method ('getPosts'). Future> getPosts(); ^ ``` ``` Analyzing example... error • lib\models\post_api.chopper.dart:21:22 • '_$PostApi.getPosts' ('Future> Function()') isn't a valid override of 'PostApi.getPosts' ('Future> Function()'). • invalid_override - The member being overridden at lib\models\post_api.chopper.dart:21:17. error • test\example_test.dart:1:8 • Target of URI doesn't exist: 'package:example/example.dart'. Try creating the file referenced by the URI, or try using a URI for a file that does exist. • uri_does_not_exist error • test\example_test.dart:6:12 • The function 'calculate' isn't defined. Try importing the library that defines 'calculate', correcting the name to the name of an existing function, or defining a function named 'calculate'. • undefined_function info • lib\models\post.g.dart:16:22 • The declaration '_$PostToJson' isn't referenced. Try removing the declaration of '_$PostToJson'. • unused_element 4 issues found. ``` ``` Dart SDK version: 3.5.1 (stable) (Tue Aug 13 21:02:17 2024 +0000) on "windows_x64" ```
techouse commented 2 months ago

As a workaround, could you call your model Article or something similar?

alvi-se commented 2 months ago

Yes, probably it's the best workaround to use both classes without conflicts.

techouse commented 2 months ago

Other options are:

  1. using a partial import
    import 'package:chopper/chopper.dart' hide Post; 
  2. importing chopper using a prefix
    import 'package:chopper/chopper.dart' as chopper; 

    We're unlikely to change any of the annotation classes (@Get(), @Post(), @Put(), @Patch(), @Delete()) becuase that would break the API.

alvi-se commented 2 months ago

Unfortunately, importing chopper with a prefix gives the same problem: the code is generated using chopper classes without the prefixes. IMHO, the best way to fix this is to make the generator add possible prefixes. This is just an idea, I am a beginner in Dart and I don't know anything about code generation, so I don't know if it's possible to do so.

techouse commented 2 months ago

the best way to fix this is to make the generator add possible prefixes

I'm just looking into that. Not sure about it either. 🙈

I suggest you don't use these reserved class names.

EDIT: Looks like freezed has a similar issue. Could be nasty.

Related: https://github.com/dart-lang/source_gen/issues/462

Guldem commented 2 months ago

Indeed looks like a tough problem. Have been experimenting a bit with macros but having checked this use case but maybe the dart augmentation offer a workaround. But this solution will have to wait till macros are ready. (And implemented in chopper)

techouse commented 2 months ago

Another workaround would be to rename @Get(), @Post(), @Put() etc. to @GET(), @POST(), @PUT() etc., but that's just kicking the can down the road, not to mention the breaking changes. 🫠