Skip to content

Commit fdabe40

Browse files
committed
feat: implement option to disable built-in conflicts in conflictingmarkers linter and add corresponding tests
1 parent 5c8ef35 commit fdabe40

File tree

6 files changed

+126
-12
lines changed

6 files changed

+126
-12
lines changed

pkg/analysis/conflictingmarkers/analyzer.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,18 @@ func newAnalyzer(cfg *ConflictingMarkersConfig) *analysis.Analyzer {
6363
}
6464
}
6565

66+
var conflictSets []ConflictSet
67+
68+
// Add built-in conflicts unless disabled
69+
if !cfg.DisableBuiltInConflicts {
70+
conflictSets = append(conflictSets, defaultConflictSets()...)
71+
}
72+
73+
// Add custom conflicts
74+
conflictSets = append(conflictSets, cfg.CustomConflicts...)
75+
6676
a := &analyzer{
67-
conflictSets: append(defaultConflictSets(), cfg.CustomConflicts...),
77+
conflictSets: conflictSets,
6878
}
6979

7080
return &analysis.Analyzer{

pkg/analysis/conflictingmarkers/analyzer_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,28 @@ func TestCustomConfiguration(t *testing.T) {
5858

5959
analysistest.Run(t, testdata, analyzer, "b")
6060
}
61+
62+
func TestDisableBuiltInConflicts(t *testing.T) {
63+
testdata := analysistest.TestData()
64+
65+
config := &conflictingmarkers.ConflictingMarkersConfig{
66+
DisableBuiltInConflicts: true,
67+
CustomConflicts: []conflictingmarkers.ConflictSet{
68+
{
69+
Name: "custom_conflict",
70+
SetA: []string{"custom:marker1", "custom:marker2"},
71+
SetB: []string{"custom:marker3", "custom:marker4"},
72+
Description: "Custom markers conflict with each other",
73+
},
74+
},
75+
}
76+
77+
initializer := conflictingmarkers.Initializer()
78+
79+
analyzer, err := initializer.Init(config)
80+
if err != nil {
81+
t.Fatal(err)
82+
}
83+
84+
analysistest.Run(t, testdata, analyzer, "c")
85+
}

pkg/analysis/conflictingmarkers/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ type ConflictingMarkersConfig struct {
2020
// CustomConflicts allows users to define their own sets of conflicting markers.
2121
// Each entry defines a conflict between two sets of markers.
2222
CustomConflicts []ConflictSet `json:"customConflicts"`
23+
24+
// DisableBuiltInConflicts allows users to opt-out of built-in conflict detection.
25+
// When set to true, only custom conflicts defined in CustomConflicts will be checked.
26+
// Built-in conflicts include optional_vs_required and default_vs_required checks.
27+
DisableBuiltInConflicts bool `json:"disableBuiltInConflicts"`
2328
}
2429

2530
// ConflictSet represents a conflict between two sets of markers.

pkg/analysis/conflictingmarkers/doc.go

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,40 +18,74 @@ limitations under the License.
1818
conflictingmarkers is a linter that detects and reports when mutually exclusive markers are used on the same field.
1919
This prevents common configuration errors and unexpected behavior in Kubernetes API types.
2020
21+
How it works:
2122
The linter reports issues when markers from both sets of a conflict pair are present on the same field.
2223
It does NOT report issues when multiple markers from the same set are present - only when markers from
2324
different sets within the same conflict definition are found together.
2425
26+
Built-in conflicts:
2527
The linter includes built-in checks for the following conflicts:
2628
2729
1. Optional vs Required:
28-
- Set A: optional, +kubebuilder:validation:Optional
29-
- Set B: required, +kubebuilder:validation:Required
30+
- Set A: +optional, +kubebuilder:validation:Optional, +k8s:optional
31+
- Set B: +required, +kubebuilder:validation:Required, +k8s:required
3032
- A field cannot be both optional and required
31-
- Note: Multiple optional markers or multiple required markers are allowed
3233
3334
2. Default vs Required:
34-
- Set A: default, +kubebuilder:default
35-
- Set B: required, +kubebuilder:validation:Required
35+
- Set A: +default, +kubebuilder:default
36+
- Set B: +required, +kubebuilder:validation:Required, +k8s:required
3637
- A field with a default value cannot be required
37-
- Note: Multiple default markers or multiple required markers are allowed
3838
39+
Configuration:
3940
The linter is configurable and allows users to define their own custom sets of conflicting markers.
4041
Each custom conflict set must specify a name, two sets of markers, and a description of why they conflict.
4142
42-
Example configuration:
43+
Configuration options:
44+
- disableBuiltInConflicts (bool): When set to true, only custom conflicts will be checked
45+
- customConflicts (array): List of custom conflict definitions
46+
47+
Example configurations:
48+
49+
1. Default behavior (built-in + custom conflicts):
50+
```yaml
51+
lintersConfig:
52+
53+
conflictingmarkers:
54+
customConflicts:
55+
- name: "deprecated_vs_experimental"
56+
setA: ["+deprecated"]
57+
setB: ["+experimental"]
58+
description: "A field cannot be both deprecated and experimental"
59+
60+
```
61+
62+
2. Custom conflicts only (built-in conflicts disabled):
4363
```yaml
4464
lintersConfig:
4565
4666
conflictingmarkers:
67+
disableBuiltInConflicts: true
4768
customConflicts:
48-
- name: "my_custom_conflict"
49-
setA: ["custom:marker1", "custom:marker2"]
50-
setB: ["custom:marker3", "custom:marker4"]
51-
description: "These markers conflict because..."
69+
- name: "internal_vs_public"
70+
setA: ["+internal", "+private"]
71+
setB: ["+public", "+external"]
72+
description: "Internal and public markers are mutually exclusive"
73+
- name: "readonly_vs_mutable"
74+
setA: ["+readonly", "+immutable"]
75+
setB: ["+mutable", "+writable"]
76+
description: "Read-only and mutable markers cannot be used together"
77+
78+
```
79+
80+
3. Built-in conflicts only (no custom conflicts):
81+
```yaml
82+
lintersConfig:
83+
84+
conflictingmarkers: {} # Uses default built-in conflicts only
5285
5386
```
5487
88+
Limitations:
5589
The linter does not provide automatic fixes as it cannot determine which conflicting marker should be removed.
5690
*/
5791
package conflictingmarkers
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package c
2+
3+
type DisableBuiltInConflictsStruct struct {
4+
// This field has built-in conflicts but they should be ignored when DisableBuiltInConflicts is true
5+
// +optional
6+
// +required
7+
BuiltInConflictIgnoredField string `json:"builtInConflictIgnoredField"`
8+
9+
// This field has custom conflicts that should still be detected
10+
// +custom:marker1
11+
// +custom:marker3
12+
CustomConflictDetectedField string `json:"customConflictDetectedField"` // want "field CustomConflictDetectedField has conflicting markers: custom_conflict: \\[custom:marker1\\] and \\[custom:marker3\\]. Custom markers conflict with each other"
13+
14+
// This field has both built-in and custom conflicts, but only custom conflicts should be reported
15+
// +optional
16+
// +required
17+
// +custom:marker1
18+
// +custom:marker3
19+
MixedConflictsField string `json:"mixedConflictsField"` // want "field MixedConflictsField has conflicting markers: custom_conflict: \\[custom:marker1\\] and \\[custom:marker3\\]. Custom markers conflict with each other"
20+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package c
2+
3+
type DisableBuiltInConflictsStruct struct {
4+
// This field has built-in conflicts but they should be ignored when DisableBuiltInConflicts is true
5+
// +optional
6+
// +required
7+
BuiltInConflictIgnoredField string `json:"builtInConflictIgnoredField"`
8+
9+
// This field has custom conflicts that should still be detected
10+
// +custom:marker1
11+
// +custom:marker3
12+
CustomConflictDetectedField string `json:"customConflictDetectedField"` // want "field CustomConflictDetectedField has conflicting markers: custom_conflict: \\[custom:marker1\\] and \\[custom:marker3\\]. Custom markers conflict with each other"
13+
14+
// This field has both built-in and custom conflicts, but only custom conflicts should be reported
15+
// +optional
16+
// +required
17+
// +custom:marker1
18+
// +custom:marker3
19+
MixedConflictsField string `json:"mixedConflictsField"` // want "field MixedConflictsField has conflicting markers: custom_conflict: \\[custom:marker1\\] and \\[custom:marker3\\]. Custom markers conflict with each other"
20+
}

0 commit comments

Comments
 (0)