Skip to content

Conversation

@andig
Copy link
Member

@andig andig commented Jan 1, 2026

@Maschga the parsing logic already exists but it's not used in the generator template. So more work to be done, feel free to take a stab.

@evcc-bot evcc-bot added the infrastructure Basic functionality label Jan 1, 2026
@andig andig added the backlog Things to do later label Jan 1, 2026
@andig andig mentioned this pull request Jan 1, 2026
@Maschga
Copy link
Collaborator

Maschga commented Jan 3, 2026

Is there anything else that needs to be done here?

@andig andig marked this pull request as ready for review January 3, 2026 10:17
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In main, when building dynamicTypes, strings.SplitN(v, ",", 2) is assumed to always return a slice of length 2; consider checking the length before indexing split[1] to avoid panics on malformed -t arguments.
  • parseFunctions silently drops a trailing token if the interface string has an odd number of comma-separated parts (e.g. missing a signature); consider validating the input and failing fast or logging an error when the function/signature pairs are incomplete.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `main`, when building `dynamicTypes`, `strings.SplitN(v, ",", 2)` is assumed to always return a slice of length 2; consider checking the length before indexing `split[1]` to avoid panics on malformed `-t` arguments.
- `parseFunctions` silently drops a trailing token if the interface string has an odd number of comma-separated parts (e.g. missing a signature); consider validating the input and failing fast or logging an error when the function/signature pairs are incomplete.

## Individual Comments

### Comment 1
<location> `cmd/decorate/decorate.go:310-314` </location>
<code_context>
+	return res
+}
+
+func parseFunctions(iface string) []function {
+	parts := splitTopLevel(iface)
+
+	var res []function
+	for i := 0; i+1 < len(parts); i += 2 {
+		res = append(res, function{
+			function:  parts[i],
</code_context>

<issue_to_address>
**issue:** parseFunctions silently drops a trailing unmatched token when the number of parts is odd

Because the loop uses `i+1 < len(parts)`, an odd number of elements from `splitTopLevel` causes the final element to be silently ignored. For malformed input like `Multi1,func() error,Multi2`, the trailing `"Multi2"` is dropped without any warning. Consider detecting `len(parts)%2 != 0` and either returning an error or otherwise surfacing this case so misconfiguration doesn’t lead to partially generated decorators.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@andig andig enabled auto-merge (squash) January 3, 2026 12:22
@andig andig disabled auto-merge January 3, 2026 12:22
@andig andig enabled auto-merge (squash) January 3, 2026 12:22
@andig andig disabled auto-merge January 3, 2026 12:23
@andig andig enabled auto-merge (squash) January 3, 2026 12:25
@andig andig merged commit 0c46b45 into master Jan 3, 2026
6 of 7 checks passed
@andig andig deleted the chore/decorate branch January 3, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backlog Things to do later infrastructure Basic functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants