Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions json_serializable/lib/src/type_helper_ctx.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/dart/constant/value.dart';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is NOT ideal...

import 'package:analyzer/dart/element/type.dart';
import 'package:source_helper/source_helper.dart';

Expand Down Expand Up @@ -122,12 +123,19 @@ ConvertData? _convertData(DartObject obj, FieldElement element, bool isFrom) {
if (objectValue == null || objectValue.isNull) {
return null;
}

final executableElement = objectValue.toFunctionValue()!;
var executableType = executableElement.type;
var qualifiedName = executableElement.qualifiedName;
final state = (objectValue as DartObjectImpl).state;
if (((state as FunctionState).typeArguments?.length ?? 0) > 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This condition can be expressed more idiomatically using isNotEmpty.

Suggested change
if (((state as FunctionState).typeArguments?.length ?? 0) > 0) {
if ((state as FunctionState).typeArguments?.isNotEmpty == true) {

final generics = state.typeArguments!.cast<DartType>();
executableType = executableType.instantiate(generics);
qualifiedName += "<${generics.join(',')}>";
}
Comment on lines +129 to +134

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

These changes rely on internal APIs from the analyzer package (DartObjectImpl, FunctionState), which are imported via package:analyzer/src/dart/constant/value.dart. These APIs are not stable and can break this package on any analyzer update, creating a significant maintenance risk.

Could you investigate if there's an alternative approach using only public APIs to retrieve the generic type arguments for the function? If this is the only way, the dependency on analyzer's internals should be clearly documented as a known risk.


if (executableElement.formalParameters.isEmpty ||
executableElement.formalParameters.first.isNamed ||
executableElement.formalParameters.where((pe) => !pe.isOptional).length >
if (executableType.formalParameters.isEmpty ||
executableType.formalParameters.first.isNamed ||
executableType.formalParameters.where((pe) => !pe.isOptional).length >
1) {
throwUnsupported(
element,
Expand All @@ -136,8 +144,8 @@ ConvertData? _convertData(DartObject obj, FieldElement element, bool isFrom) {
);
}

final returnType = executableElement.returnType;
final argType = executableElement.formalParameters.first.type;
final returnType = executableType.returnType;
final argType = executableType.formalParameters.first.type;
if (isFrom) {
final hasDefaultValue = !jsonKeyAnnotation(
element,
Expand Down Expand Up @@ -179,5 +187,5 @@ ConvertData? _convertData(DartObject obj, FieldElement element, bool isFrom) {
}
}

return ConvertData(executableElement.qualifiedName, argType, returnType);
return ConvertData(qualifiedName, argType, returnType);
}
18 changes: 15 additions & 3 deletions json_serializable/lib/src/type_helpers/json_converter_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:json_annotation/json_annotation.dart';
import 'package:source_gen/source_gen.dart';
Expand Down Expand Up @@ -281,9 +282,20 @@ _ConverterMatch? _compatibleMatch(
ElementAnnotation? annotation,
DartObject constantValue,
) {
final converterClassElement = constantValue.type!.element as ClassElement;
var converterClassType = constantValue.type! as InterfaceType;
final converterClassElement = converterClassType.element as ClassElement;
String? genericTypeArg;

final generics = (constantValue.type as InterfaceType).typeArguments;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The converterClassType variable is already assigned constantValue.type as InterfaceType on line 285. You can reuse it here to avoid the redundant cast and access.

Suggested change
final generics = (constantValue.type as InterfaceType).typeArguments;
final generics = converterClassType.typeArguments;

if (generics.isNotEmpty) {
converterClassType = converterClassElement.instantiate(
typeArguments: generics,
nullabilitySuffix: NullabilitySuffix.none,
);
genericTypeArg = generics.join(", ");
}

final jsonConverterSuper = converterClassElement.allSupertypes
final jsonConverterSuper = converterClassType.allSupertypes
.where((e) => _jsonConverterChecker.isExactly(e.element))
.singleOrNull;

Expand All @@ -302,7 +314,7 @@ _ConverterMatch? _compatibleMatch(
annotation,
constantValue,
jsonConverterSuper.typeArguments[1],
null,
genericTypeArg,
fieldType,
);
}
Expand Down
Loading