Skip to content

Commit ed3c9a0

Browse files
committed
pkg/aflow/flow/patching: harden against empty generated patches
Make codeeditor error on nop changes that don't actually change the code. Make patch testing error on empty patch. Perhaps we need a notion of "mandatory" tools that must be called successfully at least once... not sure yet.
1 parent 9a54276 commit ed3c9a0

File tree

6 files changed

+40
-3
lines changed

6 files changed

+40
-3
lines changed

pkg/aflow/action/crash/test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ func testPatch(ctx *aflow.Context, args testArgs) (testResult, error) {
4747
if err != nil {
4848
return res, err
4949
}
50+
if diff == "" {
51+
res.TestError = "No patch to test."
52+
return res, nil
53+
}
5054
res.PatchDiff = diff
5155

5256
if err := kernel.BuildKernel(args.KernelScratchSrc, args.KernelScratchSrc, args.KernelConfig, false); err != nil {

pkg/aflow/flow/patching/patching.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ The explanation of the root cause of the bug is:
129129
130130
{{.BugExplanation}}
131131
132-
{{if .TestError}}
132+
{{/* A TestError without PatchDiff means the previous invocation did not generate any patch. */}}
133+
{{if and .TestError .PatchDiff}}
133134
Another developer tried to fix this bug, and come up with the following patch:
134135
135136
{{.PatchDiff}}

pkg/aflow/template.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ func walkTemplate(n parse.Node, used map[string]bool, errp *error) {
9494
used[n.Ident[0]] = true
9595
case *parse.VariableNode:
9696
case *parse.TextNode:
97+
case *parse.IdentifierNode:
9798
default:
9899
noteError(errp, "unhandled node type %T", n)
99100
}

pkg/aflow/template_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ func TestTemplate(t *testing.T) {
6464
},
6565
err: "input foo is not provided",
6666
},
67+
{
68+
template: `{{if and .foo .bar}} yes {{end}}`,
69+
vars: map[string]reflect.Type{
70+
"foo": reflect.TypeFor[bool](),
71+
"bar": reflect.TypeFor[int](),
72+
},
73+
used: []string{"foo", "bar"},
74+
},
6775
}
6876
for i, test := range tests {
6977
t.Run(fmt.Sprint(i), func(t *testing.T) {

pkg/aflow/tool/codeeditor/codeeditor.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,11 @@ func codeeditor(ctx *aflow.Context, state state, args args) (struct{}, error) {
7979
return struct{}{}, aflow.BadCallError("CurrentCode snippet matched %v places,"+
8080
" increase context in CurrentCode to avoid ambiguity", matches)
8181
}
82-
err = osutil.WriteFile(file, slices.Concat(newLines...))
82+
newFileData := slices.Concat(newLines...)
83+
if bytes.Equal(fileData, newFileData) {
84+
return struct{}{}, aflow.BadCallError("The edit does not change the code.")
85+
}
86+
err = osutil.WriteFile(file, newFileData)
8387
return struct{}{}, err
8488
}
8589

@@ -97,7 +101,7 @@ func replace(lines, src, dst [][]byte, fuzzy bool) (newLines [][]byte, matches i
97101
si++
98102
continue
99103
}
100-
if len(l) == 0 {
104+
if len(l) == 0 && li != i {
101105
li++
102106
continue
103107
}

pkg/aflow/tool/codeeditor/codeeditor_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,25 @@ foo`)
105105
)
106106
}
107107

108+
func TestCodeeditorNopEdit(t *testing.T) {
109+
dir := writeTestFile(t, "src.c", `
110+
line0
111+
line1
112+
`)
113+
aflow.TestTool(t, Tool,
114+
state{
115+
KernelScratchSrc: dir,
116+
},
117+
args{
118+
SourceFile: "src.c",
119+
CurrentCode: " line0",
120+
NewCode: "line0",
121+
},
122+
struct{}{},
123+
`The edit does not change the code.`,
124+
)
125+
}
126+
108127
func TestCodeeditorReplacement(t *testing.T) {
109128
type Test struct {
110129
curFile string

0 commit comments

Comments
 (0)