Skip to content

Commit

Permalink
Do not generate duplicated nil-checks on assignments
Browse files Browse the repository at this point in the history
  • Loading branch information
K-Phoen committed Jun 25, 2024
1 parent 500a86b commit dfaa30c
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 24 deletions.
4 changes: 4 additions & 0 deletions internal/ast/buildervisitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 22 additions & 1 deletion internal/languages/nilchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{}{}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ export class SomeStructBuilder implements cog.Builder<sandbox.SomeStruct> {
};
}
this.internal.time.from = from;
if (!this.internal.time) {
this.internal.time = {
from: "now-6h",
to: "now",
};
}
this.internal.time.to = to;
return this;
}
Expand Down

0 comments on commit dfaa30c

Please sign in to comment.