Templates: improve required and advanced handling

[andig]

Fix #26362

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• In MarshalJSON, consider deriving pp.Required and pp.Advanced via the IsRequired() and IsAdvanced() methods instead of manually toggling the fields so the JSON representation automatically stays in sync if the logic of those helpers changes.
• The condition in RenderResult (renderMode == RenderModeUnitTest || renderMode == RenderModeInstance && !testing.Testing()) relies on operator precedence; adding explicit parentheses around the OR branches would make the intended logic clearer and less error-prone.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `MarshalJSON`, consider deriving `pp.Required` and `pp.Advanced` via the `IsRequired()` and `IsAdvanced()` methods instead of manually toggling the fields so the JSON representation automatically stays in sync if the logic of those helpers changes.
- The condition in `RenderResult` (`renderMode == RenderModeUnitTest || renderMode == RenderModeInstance && !testing.Testing()`) relies on operator precedence; adding explicit parentheses around the OR branches would make the intended logic clearer and less error-prone.

## Individual Comments

### Comment 1
<location> `util/templates/types_test.go:11-14` </location>
<code_context>
+)
+
+func TestParamLogic(t *testing.T) {
+	require.False(t, (&Param{
+		Advanced: true,
+		Required: true,
+	}).IsAdvanced(), "can't be advanced when required")
+
+	require.False(t, (&Param{
</code_context>

<issue_to_address>
**suggestion (testing):** Add positive coverage for `IsAdvanced` when a param is advanced and not required

Currently we only test that `IsAdvanced` is false when both `Advanced` and `Required` are true. Please also add cases asserting that `IsAdvanced` is true when `Advanced` is true and `Required` is false, and false when `Advanced` is false, so all `Advanced && !Required` combinations are covered.
</issue_to_address>

### Comment 2
<location> `util/templates/types_test.go:21-27` </location>
<code_context>
+		Required:   true,
+	}).IsRequired(), "can't be required when deprecated")
+
+	b, err := json.Marshal(Param{
+		Deprecated: true,
+		Advanced:   true, // omitempty
+		Required:   true, // omitempty
+	})
+	require.NoError(t, err)
+	require.Equal(t, `{"Name":"","Description":{},"Help":{},"Deprecated":true,"Type":"String"}`, string(b))
+}
</code_context>

<issue_to_address>
**suggestion (testing):** Extend `MarshalJSON` tests to cover non-deprecated required params where `Advanced` is forced to false

The JSON test currently only covers the deprecated + required case and implicitly checks `Advanced` via `Required` being cleared. The updated `MarshalJSON` also forces `Advanced` to false when `Required` is true. Please add a test for a non-deprecated param with `Required: true, Advanced: true` to confirm the marshaled JSON omits `Advanced` or sets it to false, so the second branch of the new logic is explicitly exercised.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[andig]

@naltatis not sure why integration doesn't like this?

[naltatis]

This change produces invalid json response:

[naltatis]

I'd prefer to not introduce this logic but implement a build time check that catches this and fails our pipeline. We already have this for other aspects. I'll check ....

[naltatis]

See here

evcc/util/templates/template.go

Lines 67 to 80 in 5262036

	// Validate that a param cannot be both masked and private
	if p.Mask && p.Private {
	return fmt.Errorf("param %s: 'mask' and 'private' cannot be used together. Use 'mask' for sensitive data like passwords/tokens that should be hidden in UI. Use 'private' for personal data like emails/locations that should only be redacted from bug reports", p.Name)
	}
	if p.Description.String("en") == "" || p.Description.String("de") == "" {
	return fmt.Errorf("param %s: description can't be empty", p.Name)
	}
	maxLength := 50
	actualLength := max(len(p.Description.String("en")), len(p.Description.String("de")))
	if actualLength > maxLength {
	return fmt.Errorf("param %s: description too long (%d/%d allowed)- use help instead", p.Name, actualLength, maxLength)
	}

[andig]

Feel free to open separate PR and see how wide-spread the issue is.

[andig]

I don‘t see any invalid json?

[naltatis]

See screenshot. The descriptions (previously strings) now return objects.

invalid meaning unexpected structure. not syntax

[andig]

Wow. Das war die Vorhölle aus https://deepsource.com/blog/custom-marshalers-in-go. Go schafft es immer wieder, mich (negativ) zu überraschen. Fixed.