From dfaa30c9416e6cafa2e6145c15bdca5d72c971bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Gomez?= Date: Tue, 25 Jun 2024 11:20:02 +0200 Subject: [PATCH] Do not generate duplicated nil-checks on assignments --- internal/ast/buildervisitor.go | 4 ++++ internal/languages/nilchecks.go | 23 ++++++++++++++++++- .../sandbox/somestruct_builder_gen.go | 6 ----- .../JavaBuilders/sandbox/SomeStruct.java | 3 --- .../src/Sandbox/SomeStructBuilder.php | 6 +---- .../PythonBuilder/builders/sandbox.py | 3 --- .../src/sandbox/someStructBuilder.gen.ts | 6 ----- 7 files changed, 27 insertions(+), 24 deletions(-) diff --git a/internal/ast/buildervisitor.go b/internal/ast/buildervisitor.go index d788a22b..4acf4f54 100644 --- a/internal/ast/buildervisitor.go +++ b/internal/ast/buildervisitor.go @@ -90,6 +90,10 @@ func (visitor *BuilderVisitor) VisitConstructor(schemas Schemas, builder Builder return visitor.OnConstructor(visitor, schemas, builder, constructor) } + return visitor.TraverseConstructor(schemas, builder, constructor) +} + +func (visitor *BuilderVisitor) TraverseConstructor(schemas Schemas, builder Builder, constructor Constructor) (Constructor, error) { var arg Argument var assign Assignment var err error diff --git a/internal/languages/nilchecks.go b/internal/languages/nilchecks.go index 6aa73faa..5c0a5073 100644 --- a/internal/languages/nilchecks.go +++ b/internal/languages/nilchecks.go @@ -15,8 +15,23 @@ func GenerateBuilderNilChecks(language Language, context Context) (Context, erro nullableKinds = nilTypesProvider.NullableKinds() } + // Allows us to keep track of the checks already performed for the current scope (constructor or option) + // When a check is generated, the path being checked is stored in this map. + // Changes in scope must reset this map. + checks := make(map[string]struct{}) + nilChecksVisitor := ast.BuilderVisitor{ - OnAssignment: func(_ *ast.BuilderVisitor, _ ast.Schemas, _ ast.Builder, assignment ast.Assignment) (ast.Assignment, error) { + OnConstructor: func(visitor *ast.BuilderVisitor, schemas ast.Schemas, builder ast.Builder, constructor ast.Constructor) (ast.Constructor, error) { + checks = make(map[string]struct{}) + + return visitor.TraverseConstructor(schemas, builder, constructor) + }, + OnOption: func(visitor *ast.BuilderVisitor, schemas ast.Schemas, builder ast.Builder, option ast.Option) (ast.Option, error) { + checks = make(map[string]struct{}) + + return visitor.TraverseOption(schemas, builder, option) + }, + OnAssignment: func(_ *ast.BuilderVisitor, _ ast.Schemas, b ast.Builder, assignment ast.Assignment) (ast.Assignment, error) { for i, chunk := range assignment.Path { protectArrayAppend := nullableKinds.ProtectArrayAppend && assignment.Method == ast.AppendAssignment if i == len(assignment.Path)-1 && !protectArrayAppend { @@ -33,10 +48,16 @@ func GenerateBuilderNilChecks(language Language, context Context) (Context, erro valueType = *subPath.Last().TypeHint } + // this path already has a nil check: nothing to do. + if _, found := checks[subPath.String()]; found { + continue + } + assignment.NilChecks = append(assignment.NilChecks, ast.AssignmentNilCheck{ Path: subPath, EmptyValueType: valueType, }) + checks[subPath.String()] = struct{}{} } } diff --git a/testdata/jennies/builders/struct_fields_as_args_assignment/GoBuilder/sandbox/somestruct_builder_gen.go b/testdata/jennies/builders/struct_fields_as_args_assignment/GoBuilder/sandbox/somestruct_builder_gen.go index e8264e79..3fdfb67e 100644 --- a/testdata/jennies/builders/struct_fields_as_args_assignment/GoBuilder/sandbox/somestruct_builder_gen.go +++ b/testdata/jennies/builders/struct_fields_as_args_assignment/GoBuilder/sandbox/somestruct_builder_gen.go @@ -45,12 +45,6 @@ if builder.internal.Time == nil { }{} } builder.internal.Time.From = from -if builder.internal.Time == nil { - builder.internal.Time = &struct { - From string `json:"from"` - To string `json:"to"` -}{} -} builder.internal.Time.To = to return builder diff --git a/testdata/jennies/builders/struct_fields_as_args_assignment/JavaBuilders/sandbox/SomeStruct.java b/testdata/jennies/builders/struct_fields_as_args_assignment/JavaBuilders/sandbox/SomeStruct.java index 82af1327..322edd7e 100644 --- a/testdata/jennies/builders/struct_fields_as_args_assignment/JavaBuilders/sandbox/SomeStruct.java +++ b/testdata/jennies/builders/struct_fields_as_args_assignment/JavaBuilders/sandbox/SomeStruct.java @@ -15,9 +15,6 @@ public Builder Time(String from,String to) { this.internal.time = new Object(); } this.internal.time.from = from; - if (this.internal.time == null) { - this.internal.time = new Object(); - } this.internal.time.to = to; return this; } diff --git a/testdata/jennies/builders/struct_fields_as_args_assignment/PHPBuilder/src/Sandbox/SomeStructBuilder.php b/testdata/jennies/builders/struct_fields_as_args_assignment/PHPBuilder/src/Sandbox/SomeStructBuilder.php index cee28e05..8884cb24 100644 --- a/testdata/jennies/builders/struct_fields_as_args_assignment/PHPBuilder/src/Sandbox/SomeStructBuilder.php +++ b/testdata/jennies/builders/struct_fields_as_args_assignment/PHPBuilder/src/Sandbox/SomeStructBuilder.php @@ -28,11 +28,7 @@ public function time(string $from,string $to): static $this->internal->time = "unknown"; } - $this->internal->time->from = $from; - if ($this->internal->time === null) { - $this->internal->time = "unknown"; - } - + $this->internal->time->from = $from; $this->internal->time->to = $to; return $this; diff --git a/testdata/jennies/builders/struct_fields_as_args_assignment/PythonBuilder/builders/sandbox.py b/testdata/jennies/builders/struct_fields_as_args_assignment/PythonBuilder/builders/sandbox.py index ad044bb9..115dee55 100644 --- a/testdata/jennies/builders/struct_fields_as_args_assignment/PythonBuilder/builders/sandbox.py +++ b/testdata/jennies/builders/struct_fields_as_args_assignment/PythonBuilder/builders/sandbox.py @@ -17,9 +17,6 @@ def time(self, from_val: str, to: str) -> typing.Self: self._internal.time = "unknown" assert isinstance(self._internal.time, unknown) self._internal.time.from_val = from_val - if self._internal.time is None: - self._internal.time = "unknown" - assert isinstance(self._internal.time, unknown) self._internal.time.to = to return self diff --git a/testdata/jennies/builders/struct_fields_as_args_assignment/TypescriptBuilder/src/sandbox/someStructBuilder.gen.ts b/testdata/jennies/builders/struct_fields_as_args_assignment/TypescriptBuilder/src/sandbox/someStructBuilder.gen.ts index 2a9665be..83d4335e 100644 --- a/testdata/jennies/builders/struct_fields_as_args_assignment/TypescriptBuilder/src/sandbox/someStructBuilder.gen.ts +++ b/testdata/jennies/builders/struct_fields_as_args_assignment/TypescriptBuilder/src/sandbox/someStructBuilder.gen.ts @@ -20,12 +20,6 @@ export class SomeStructBuilder implements cog.Builder { }; } this.internal.time.from = from; - if (!this.internal.time) { - this.internal.time = { - from: "now-6h", - to: "now", -}; - } this.internal.time.to = to; return this; }