Skip to content

Commit 1e10f36

Browse files
committed
refactor(markerscope): replace StrictTypeConstraint with NamedTypeConstraint and improve code quality
Signed-off-by: nayuta-ai <nayuta723@gmail.com>
1 parent 875b36c commit 1e10f36

File tree

6 files changed

+192
-135
lines changed

6 files changed

+192
-135
lines changed

pkg/analysis/markerscope/analyzer.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,13 @@ func newAnalyzer(cfg *MarkerScopeConfig) *analysis.Analyzer {
106106
// markerRulesListToMap converts a list of marker rules to a map keyed by marker identifier.
107107
func markerRulesListToMap(rules []MarkerScopeRule) map[string]MarkerScopeRule {
108108
result := make(map[string]MarkerScopeRule, len(rules))
109+
109110
for _, rule := range rules {
110111
if rule.Identifier != "" {
111112
result[rule.Identifier] = rule
112113
}
113114
}
115+
114116
return result
115117
}
116118

@@ -120,7 +122,6 @@ func defaultConfig(cfg *MarkerScopeConfig) {
120122
if cfg.Policy == "" {
121123
cfg.Policy = MarkerScopePolicyWarn
122124
}
123-
// overrideMarkers and customMarkers default to empty (zero value)
124125
}
125126

126127
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
@@ -244,7 +245,7 @@ func (a *analyzer) checkFieldTypeConstraintViolation(pass *analysis.Pass, field
244245

245246
if a.policy == MarkerScopePolicySuggestFix {
246247
// Check if this is a "should be on type definition" error
247-
var moveErr *MarkerShouldBeOnTypeDefinitionError
248+
var moveErr *markerShouldBeOnTypeDefinitionError
248249
if errors.As(err, &moveErr) {
249250
// Suggest moving to type definition
250251
fixes = a.suggestMoveToFieldsIfCompatible(pass, field, marker, rule)
@@ -305,7 +306,7 @@ func (a *analyzer) checkTypeConstraintViolation(pass *analysis.Pass, typeSpec *a
305306
if a.policy == MarkerScopePolicySuggestFix {
306307
// Check if this is a "should be on field" error (though validateTypeSpecTypeConstraint doesn't return this)
307308
// For consistency with checkFieldMarkers, we check the error type
308-
var moveErr *MarkerShouldBeOnTypeDefinitionError
309+
var moveErr *markerShouldBeOnTypeDefinitionError
309310
if errors.As(err, &moveErr) {
310311
// This shouldn't happen for type specs, but handle it for consistency
311312
fixes = a.suggestMoveToField(pass, typeSpec, marker, rule)
@@ -347,10 +348,11 @@ func (a *analyzer) validateFieldTypeConstraint(pass *analysis.Pass, field *ast.F
347348
return err
348349
}
349350

350-
if rule.StrictTypeConstraint && rule.Scope == AnyScope {
351+
// Check if the marker should be on the type definition instead of the field
352+
if rule.NamedTypeConstraint == NamedTypeConstraintRequireTypeDefinition && rule.Scope == AnyScope {
351353
namedType, ok := tv.Type.(*types.Named)
352354
if ok {
353-
return &MarkerShouldBeOnTypeDefinitionError{TypeName: namedType.Obj().Name()}
355+
return &markerShouldBeOnTypeDefinitionError{typeName: namedType.Obj().Name()}
354356
}
355357
}
356358

@@ -385,7 +387,7 @@ func validateTypeAgainstConstraint(t types.Type, tc *TypeConstraint) error {
385387
// Check if the schema type is allowed
386388
if len(tc.AllowedSchemaTypes) > 0 {
387389
if !slices.Contains(tc.AllowedSchemaTypes, schemaType) {
388-
return &TypeNotAllowedError{Type: schemaType, AllowedTypes: tc.AllowedSchemaTypes}
390+
return &typeNotAllowedError{schemaType: schemaType, allowedTypes: tc.AllowedSchemaTypes}
389391
}
390392
}
391393

@@ -394,7 +396,7 @@ func validateTypeAgainstConstraint(t types.Type, tc *TypeConstraint) error {
394396
elemType := getElementType(t)
395397
if elemType != nil {
396398
if err := validateTypeAgainstConstraint(elemType, tc.ElementConstraint); err != nil {
397-
return &InvalidElementConstraintError{Err: err}
399+
return &invalidElementConstraintError{err: err}
398400
}
399401
}
400402
}

pkg/analysis/markerscope/analyzer_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func TestAnalyzerWithDefaultConfig(t *testing.T) {
3232
if err != nil {
3333
t.Fatal(err)
3434
}
35+
3536
analysistest.Run(t, testdata, analyzer, "a")
3637
}
3738

@@ -40,10 +41,12 @@ func TestAnalyzerSuggestFixes(t *testing.T) {
4041
cfg := &markerscope.MarkerScopeConfig{
4142
Policy: markerscope.MarkerScopePolicySuggestFix,
4243
}
44+
4345
analyzer, err := markerscope.Initializer().Init(cfg)
4446
if err != nil {
4547
t.Fatal(err)
4648
}
49+
4750
analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "a")
4851
}
4952

@@ -111,9 +114,11 @@ func TestAnalyzerWithCustomAndOverrideMarkers(t *testing.T) {
111114
},
112115
},
113116
}
117+
114118
analyzer, err := markerscope.Initializer().Init(cfg)
115119
if err != nil {
116120
t.Fatal(err)
117121
}
122+
118123
analysistest.Run(t, testdata, analyzer, "b")
119124
}

0 commit comments

Comments
 (0)