Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,11 @@ class BuiltBindingsAnalyzer extends Analyzer<List<Binding>> {
);
}

final isIgnored = ignoredTargets.contains(targetName);
final callableMappingMethod = callableMap[targetName];
final extraMappingMethod = callableMappingMethod == null
// Ignored targets must not synthesize a nested converter: analyzing
// it would surface binding errors for a mapping that is never emitted.
final extraMappingMethod = callableMappingMethod == null && !isIgnored
? extraMappingMethodAnalyzer.analyze(
FieldsAnalyzerContext(
mapperAnnotation: context.mapperAnnotation,
Expand All @@ -169,7 +172,7 @@ class BuiltBindingsAnalyzer extends Analyzer<List<Binding>> {
Binding(
source: resolvedField,
target: targetField,
ignored: ignoredTargets.contains(targetName),
ignored: isIgnored,
forceNonNull: forceNonNullTargets.contains(targetName),
callableMappingMethod: callableMappingMethod,
extraMappingMethod: extraMappingMethod,
Expand Down Expand Up @@ -253,8 +256,11 @@ class BuiltBindingsAnalyzer extends Analyzer<List<Binding>> {
nullable: (targetSubstituted?[targetName] ?? targetGetter.type).isNullable,
);

final isIgnored = ignoredTargets.contains(targetName);
final callableMappingMethod = callableMap[targetName];
final extraMappingMethod = callableMappingMethod == null
// Ignored targets must not synthesize a nested converter: analyzing
// it would surface binding errors for a mapping that is never emitted.
final extraMappingMethod = callableMappingMethod == null && !isIgnored
? extraMappingMethodAnalyzer.analyze(
FieldsAnalyzerContext(
mapperAnnotation: context.mapperAnnotation,
Expand All @@ -272,7 +278,7 @@ class BuiltBindingsAnalyzer extends Analyzer<List<Binding>> {
Binding(
source: sourceField,
target: targetField,
ignored: ignoredTargets.contains(targetName),
ignored: isIgnored,
forceNonNull: forceNonNullTargets.contains(targetName),
callableMappingMethod: callableMappingMethod,
extraMappingMethod: extraMappingMethod,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ class StandardBindingsAnalyzer extends Analyzer<List<Binding>> {
// and the target is non-null, and forceNonNull is not set, throw.
final chainHasNullable = accessChain.any((e) => e.$2);
final resolvedIsNullable = resolvedField.nullable || chainHasNullable;
final isIgnored = ignoredTargets.contains(targetName);
final callableMappingMethod = callableMap[targetName];
if (resolvedIsNullable &&
!targetField.nullable &&
Expand All @@ -312,7 +313,9 @@ class StandardBindingsAnalyzer extends Analyzer<List<Binding>> {
element: method,
);
}
final extraMappingMethod = callableMappingMethod == null
// Ignored targets must not synthesize a nested converter: analyzing
// it would surface binding errors for a mapping that is never emitted.
final extraMappingMethod = callableMappingMethod == null && !isIgnored
? extraMappingMethodAnalyzer.analyze(
FieldsAnalyzerContext(
mapperAnnotation: context.mapperAnnotation,
Expand All @@ -330,7 +333,7 @@ class StandardBindingsAnalyzer extends Analyzer<List<Binding>> {
Binding(
source: resolvedField,
target: targetField,
ignored: ignoredTargets.contains(targetName),
ignored: isIgnored,
forceNonNull: forceNonNullTargets.contains(targetName),
callableMappingMethod: callableMappingMethod,
extraMappingMethod: extraMappingMethod,
Expand Down Expand Up @@ -476,8 +479,11 @@ class StandardBindingsAnalyzer extends Analyzer<List<Binding>> {
nullable: (targetSubstituted?[targetClassParamName] ?? resolvedTargetParam.type).isNullable,
);

final isIgnored = ignoredTargets.contains(targetClassParamName);
final callableMappingMethod = callableMap[targetClassParamName];
final extraMappingMethod = callableMappingMethod == null
// Ignored targets must not synthesize a nested converter: analyzing
// it would surface binding errors for a mapping that is never emitted.
final extraMappingMethod = callableMappingMethod == null && !isIgnored
Comment thread
coderabbitai[bot] marked this conversation as resolved.
? extraMappingMethodAnalyzer.analyze(
FieldsAnalyzerContext(
mapperAnnotation: context.mapperAnnotation,
Expand All @@ -495,7 +501,7 @@ class StandardBindingsAnalyzer extends Analyzer<List<Binding>> {
Binding(
source: sourceField,
target: targetField,
ignored: ignoredTargets.contains(targetClassParamName),
ignored: isIgnored,
forceNonNull:
forceNonNullTargets.contains(targetClassParamName),
callableMappingMethod: callableMappingMethod,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@ class EnumExpressionFactory extends ExpressionFactory {

@override
Expression create(ExpressionContext context) {
// The <NULL> sentinel is a target value, not an identifier: it must resolve
// to `null` regardless of the target type (String, num, enum, ...).
if (context.origin == FieldOrigin.target &&
context.field.name == ValueMapping.nullValue) {
return literalNull;
}

if (context.field.type.isPrimitive) {
if (context.origin == FieldOrigin.target) {
if (context.field.name == ValueMapping.nullValue) {
return literalNull;
}

if (context.field.type.isDartCoreInt) {
return literal(context.field.name).stringToInt(
nullable: context.currentMethod.optionalReturn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/

import 'package:code_builder/code_builder.dart';
import 'package:dart_mapper/dart_mapper.dart';
import 'package:code_builder/code_builder.dart' hide Field;
import 'package:dart_mapper_generator/src/exceptions/unknown_return_type_error.dart';
import 'package:dart_mapper_generator/src/extensions/element.dart';
import 'package:dart_mapper_generator/src/factories/expression_factory.dart';
import 'package:dart_mapper_generator/src/misc/expressions.dart';
import 'package:dart_mapper_generator/src/misc/strings.dart';
import 'package:dart_mapper_generator/src/models/field/field.dart';
import 'package:dart_mapper_generator/src/models/mapper/mapping/method/bases/bindable_mapping_method.dart';
import 'package:dart_mapper_generator/src/models/mapper/mapping/method/defined_mapping_method.dart';
import 'package:dart_mapper_generator/src/models/mapping_behavior.dart';
Expand Down Expand Up @@ -65,10 +65,26 @@ class EnumMappingCodeProcessor extends ComponentProcessor<Code> {

final safeEnumDisplayName =
targetEnum.displayName.replaceAll('\\', '\\\\').replaceAll(r'$', r'\$');
final qualifiedEnumName = context.resolveType(method.returnType!);
final sourceField = method.parameters.first.field;
final expressionFactory = expressionStrategyDispatcher.get(method.behavior);

// Sentinel-driven cases (<NULL> source, <ANY_REMAINING>, <ANY_UNMAPPED>)
// carry a target *name* rather than a Binding. Route them through the
// expression factory like every other case, so the target type — enum,
// String, num — decides how the value is rendered.
Expression targetExpression(String targetName) => expressionFactory.create(
ExpressionContext(
field: Field.from(
name: targetName,
type: method.returnType!,
),
origin: FieldOrigin.target,
counterpartField: sourceField,
currentMethod: method,
importAliases: context.importAliases,
),
);

return Block(
(b) => b
..addExpression(
Expand All @@ -79,7 +95,7 @@ class EnumMappingCodeProcessor extends ComponentProcessor<Code> {
(
literal(null),
method is DefinedMappingMethod && method.nullSourceTarget != null
? refer(qualifiedEnumName).property(method.nullSourceTarget!)
? targetExpression(method.nullSourceTarget!)
: method.optionalReturn
? literal(null)
: throwArgumentErrorNotNull(sourceField.name),
Expand Down Expand Up @@ -111,8 +127,8 @@ class EnumMappingCodeProcessor extends ComponentProcessor<Code> {
otherwise: _buildOtherwiseExpression(
method: method,
safeEnumDisplayName: safeEnumDisplayName,
qualifiedEnumName: qualifiedEnumName,
sourceFieldName: sourceField.name,
targetExpression: targetExpression,
),
).returned,
),
Expand All @@ -122,18 +138,15 @@ class EnumMappingCodeProcessor extends ComponentProcessor<Code> {
Expression _buildOtherwiseExpression({
required BindableMappingMethod method,
required String safeEnumDisplayName,
required String qualifiedEnumName,
required String sourceFieldName,
required Expression Function(String targetName) targetExpression,
}) {
if (method is DefinedMappingMethod && method.anyRemainingTarget != null) {
return refer(qualifiedEnumName).property(method.anyRemainingTarget!);
return targetExpression(method.anyRemainingTarget!);
}

if (method is DefinedMappingMethod && method.anyUnmappedTarget != null) {
if (method.anyUnmappedTarget == ValueMapping.nullValue) {
return literal(null);
}
return refer(qualifiedEnumName).property(method.anyUnmappedTarget!);
return targetExpression(method.anyUnmappedTarget!);
}

if (method.optionalReturn) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,53 @@ abstract class CallableNullableDotNotationMapper {
@Mapping(target: 'streetName', source: 'address.street.name', callable: _upperCase)
FlatCallableTarget flattenCallable(NullableStreetPerson source);
}

// Regression: `ignore: true` combined with a dot-notation source must suppress
// synthesis of the nested converter. The explicit-mapping branch used to
// analyze the extra mapping method regardless of ignoredTargets, throwing
// NoRelationFoundError for a converter that is never emitted.

class IgnoredDotInnerSource {
final String code;

const IgnoredDotInnerSource(this.code);
}

class IgnoredDotInnerTarget {
final String code;
final String direction;

const IgnoredDotInnerTarget({required this.code, required this.direction});
}

class IgnoredDotWrapper {
final IgnoredDotInnerSource? inner;

const IgnoredDotWrapper(this.inner);
}

class IgnoredDotSource {
final String id;
final IgnoredDotWrapper wrapper;

const IgnoredDotSource(this.id, this.wrapper);
}

class IgnoredDotTarget {
final String id;
final IgnoredDotInnerTarget? inner;

const IgnoredDotTarget({required this.id, this.inner});
}

@ShouldGenerate(
r'''IgnoredDotTarget map(IgnoredDotSource source) {
return IgnoredDotTarget(id: source.id, inner: null);
}''',
contains: true,
)
@Mapper()
abstract class IgnoredDotNotationMapper {
@Mapping(target: 'inner', source: 'wrapper.inner', ignore: true)
IgnoredDotTarget map(IgnoredDotSource source);
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,41 @@ abstract class MutualExclusionEnumMapper {
@ValueMapping(source: ValueMapping.anyUnmapped, target: ValueMapping.nullValue)
TargetColor convert(SourceColor source);
}

// Regression: the fallback target must be rendered by the expression factory,
// not as `refer(returnType).property(target)`. With a non-enum return type the
// latter emitted `String.UNKNOWN` (parsable but wrong) and `int.-1` (a build
// failure).

@ShouldGenerate(
r"""_ => 'UNKNOWN',""",
contains: true,
)
@Mapper()
abstract class AnyRemainingToStringMapper {
@ValueMapping(source: ValueMapping.anyRemaining, target: 'UNKNOWN')
@ValueMapping(source: 'red', target: 'RED')
String convert(SourceColor source);
}

@ShouldGenerate(
r'''_ => int.parse('-1'),''',
contains: true,
)
@Mapper()
abstract class AnyRemainingToIntMapper {
@ValueMapping(source: ValueMapping.anyRemaining, target: '-1')
@ValueMapping(source: 'red', target: '0')
int convert(SourceColor source);
}

@ShouldGenerate(
r'''_ => null,''',
contains: true,
)
@Mapper()
abstract class AnyUnmappedToStringMapper {
@ValueMapping(source: ValueMapping.anyUnmapped, target: ValueMapping.nullValue)
@ValueMapping(source: 'red', target: 'RED')
String? convert(SourceColor source);
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,51 @@ abstract class InnerMapper {
abstract class OuterMapper {
OuterTarget toTarget(OuterSource source);
}

// Regression: `ignore: true` on a nested field must suppress synthesis of the
// nested converter. The extra mapping method used to be analyzed anyway,
// throwing NoRelationFoundError for a mapping that is never emitted.

class IgnoredInnerSource {
final String code;

IgnoredInnerSource(this.code);
}

class IgnoredInnerTarget {
final String code;
final String direction;

IgnoredInnerTarget({required this.code, required this.direction});
}

class IgnoredNestedSource {
final String id;
final IgnoredInnerSource? inner;

IgnoredNestedSource(this.id, this.inner);
}

class IgnoredNestedTarget {
final String id;
final IgnoredInnerTarget? inner;

IgnoredNestedTarget({required this.id, this.inner});
}

@ShouldGenerate(
r'''class IgnoredNestedMapperImpl extends IgnoredNestedMapper {
IgnoredNestedMapperImpl();

@override
IgnoredNestedTarget toTarget(IgnoredNestedSource source) {
return IgnoredNestedTarget(id: source.id, inner: null);
}
}''',
contains: true,
)
@Mapper()
abstract class IgnoredNestedMapper {
@Mapping(target: 'inner', ignore: true)
IgnoredNestedTarget toTarget(IgnoredNestedSource source);
}
Loading