Skip to content

Commit 34b6e21

Browse files
josephschorrjosephschorr
committed
Add ability to toggle off specific warnings via magic comments
Fixes #1975
1 parent 5ed2d0f commit 34b6e21

File tree

4 files changed

+279
-166
lines changed

4 files changed

+279
-166
lines changed

‎internal/lsp/lsp_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func TestDocumentWarningDiagnostics(t *testing.T) {
100100
require.Equal(t, "full", resp.Kind)
101101
require.Len(t, resp.Items, 1)
102102
require.Equal(t, lsp.DiagnosticSeverity(lsp.Warning), resp.Items[0].Severity)
103-
require.Equal(t, `Permission "view_resource" references parent type "resource" in its name; it is recommended to drop the suffix`, resp.Items[0].Message)
103+
require.Equal(t, `Permission "view_resource" references parent type "resource" in its name; it is recommended to drop the suffix (relation-name-references-parent)`, resp.Items[0].Message)
104104
require.Equal(t, lsp.Range{
105105
Start: lsp.Position{Line: 5, Character: 3},
106106
End: lsp.Position{Line: 5, Character: 3},

‎pkg/development/warningdefs.go

+155-134
Original file line numberDiff line numberDiff line change
@@ -11,178 +11,199 @@ import (
1111
"github.com/authzed/spicedb/pkg/typesystem"
1212
)
1313

14-
var lintRelationReferencesParentType = func(
15-
ctx context.Context,
16-
relation *corev1.Relation,
17-
ts *typesystem.TypeSystem,
18-
) (*devinterface.DeveloperWarning, error) {
19-
parentDef := ts.Namespace()
20-
if strings.HasSuffix(relation.Name, parentDef.Name) {
21-
if ts.IsPermission(relation.Name) {
14+
var lintRelationReferencesParentType = relationCheck{
15+
"relation-name-references-parent",
16+
func(
17+
ctx context.Context,
18+
relation *corev1.Relation,
19+
ts *typesystem.TypeSystem,
20+
) (*devinterface.DeveloperWarning, error) {
21+
parentDef := ts.Namespace()
22+
if strings.HasSuffix(relation.Name, parentDef.Name) {
23+
if ts.IsPermission(relation.Name) {
24+
return warningForMetadata(
25+
"relation-name-references-parent",
26+
fmt.Sprintf("Permission %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
27+
relation,
28+
), nil
29+
}
30+
2231
return warningForMetadata(
23-
fmt.Sprintf("Permission %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
32+
"relation-name-references-parent",
33+
fmt.Sprintf("Relation %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
2434
relation,
2535
), nil
2636
}
2737

28-
return warningForMetadata(
29-
fmt.Sprintf("Relation %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
30-
relation,
31-
), nil
32-
}
33-
34-
return nil, nil
35-
}
36-
37-
var lintPermissionReferencingItself = func(
38-
ctx context.Context,
39-
computedUserset *corev1.ComputedUserset,
40-
sourcePosition *corev1.SourcePosition,
41-
ts *typesystem.TypeSystem,
42-
) (*devinterface.DeveloperWarning, error) {
43-
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
44-
permName := parentRelation.Name
45-
if computedUserset.Relation == permName {
46-
return warningForPosition(
47-
fmt.Sprintf("Permission %q references itself, which will cause an error to be raised due to infinite recursion", permName),
48-
sourcePosition,
49-
), nil
50-
}
51-
52-
return nil, nil
38+
return nil, nil
39+
},
5340
}
5441

55-
var lintArrowReferencingUnreachable = func(
56-
ctx context.Context,
57-
ttu *corev1.TupleToUserset,
58-
sourcePosition *corev1.SourcePosition,
59-
ts *typesystem.TypeSystem,
60-
) (*devinterface.DeveloperWarning, error) {
61-
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
42+
var lintPermissionReferencingItself = computedUsersetCheck{
43+
"permission-references-itself",
44+
func(
45+
ctx context.Context,
46+
computedUserset *corev1.ComputedUserset,
47+
sourcePosition *corev1.SourcePosition,
48+
ts *typesystem.TypeSystem,
49+
) (*devinterface.DeveloperWarning, error) {
50+
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
51+
permName := parentRelation.Name
52+
if computedUserset.Relation == permName {
53+
return warningForPosition(
54+
"permission-references-itself",
55+
fmt.Sprintf("Permission %q references itself, which will cause an error to be raised due to infinite recursion", permName),
56+
sourcePosition,
57+
), nil
58+
}
6259

63-
referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
64-
if !ok {
6560
return nil, nil
66-
}
61+
},
62+
}
6763

68-
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
69-
if err != nil {
70-
return nil, err
71-
}
64+
var lintArrowReferencingUnreachable = ttuCheck{
65+
"arrow-references-unreachable-relation",
66+
func(
67+
ctx context.Context,
68+
ttu *corev1.TupleToUserset,
69+
sourcePosition *corev1.SourcePosition,
70+
ts *typesystem.TypeSystem,
71+
) (*devinterface.DeveloperWarning, error) {
72+
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
73+
74+
referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
75+
if !ok {
76+
return nil, nil
77+
}
7278

73-
wasFound := false
74-
for _, subjectType := range allowedSubjectTypes {
75-
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
79+
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
7680
if err != nil {
7781
return nil, err
7882
}
7983

80-
_, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
81-
if ok {
82-
wasFound = true
84+
wasFound := false
85+
for _, subjectType := range allowedSubjectTypes {
86+
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
87+
if err != nil {
88+
return nil, err
89+
}
90+
91+
_, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
92+
if ok {
93+
wasFound = true
94+
}
8395
}
84-
}
85-
86-
if !wasFound {
87-
return warningForPosition(
88-
fmt.Sprintf(
89-
"Arrow `%s->%s` under permission %q references relation/permission %q that does not exist on any subject types of relation %q",
90-
ttu.Tupleset.Relation,
91-
ttu.ComputedUserset.Relation,
92-
parentRelation.Name,
93-
ttu.ComputedUserset.Relation,
94-
ttu.Tupleset.Relation,
95-
),
96-
sourcePosition,
97-
), nil
98-
}
99-
100-
return nil, nil
101-
}
102-
103-
var lintArrowOverSubRelation = func(
104-
ctx context.Context,
105-
ttu *corev1.TupleToUserset,
106-
sourcePosition *corev1.SourcePosition,
107-
ts *typesystem.TypeSystem,
108-
) (*devinterface.DeveloperWarning, error) {
109-
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
110-
111-
referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
112-
if !ok {
113-
return nil, nil
114-
}
11596

116-
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
117-
if err != nil {
118-
return nil, err
119-
}
120-
121-
for _, subjectType := range allowedSubjectTypes {
122-
if subjectType.Relation != tuple.Ellipsis {
97+
if !wasFound {
12398
return warningForPosition(
99+
"arrow-references-unreachable-relation",
124100
fmt.Sprintf(
125-
"Arrow `%s->%s` under permission %q references relation %q that has relation %q on subject %q: *the subject relation will be ignored for the arrow*",
101+
"Arrow `%s->%s` under permission %q references relation/permission %q that does not exist on any subject types of relation %q",
126102
ttu.Tupleset.Relation,
127103
ttu.ComputedUserset.Relation,
128104
parentRelation.Name,
105+
ttu.ComputedUserset.Relation,
129106
ttu.Tupleset.Relation,
130-
subjectType.Relation,
131-
subjectType.Namespace,
132107
),
133108
sourcePosition,
134109
), nil
135110
}
136-
}
137-
138-
return nil, nil
139-
}
140-
141-
var lintArrowReferencingRelation = func(
142-
ctx context.Context,
143-
ttu *corev1.TupleToUserset,
144-
sourcePosition *corev1.SourcePosition,
145-
ts *typesystem.TypeSystem,
146-
) (*devinterface.DeveloperWarning, error) {
147-
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
148111

149-
referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
150-
if !ok {
151112
return nil, nil
152-
}
113+
},
114+
}
153115

154-
// For each subject type of the referenced relation, check if the referenced permission
155-
// is, in fact, a relation.
156-
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
157-
if err != nil {
158-
return nil, err
159-
}
116+
var lintArrowOverSubRelation = ttuCheck{
117+
"arrow-walks-subject-relation",
118+
func(
119+
ctx context.Context,
120+
ttu *corev1.TupleToUserset,
121+
sourcePosition *corev1.SourcePosition,
122+
ts *typesystem.TypeSystem,
123+
) (*devinterface.DeveloperWarning, error) {
124+
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
125+
126+
referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
127+
if !ok {
128+
return nil, nil
129+
}
160130

161-
for _, subjectType := range allowedSubjectTypes {
162-
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
131+
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
163132
if err != nil {
164133
return nil, err
165134
}
166135

167-
targetRelation, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
136+
for _, subjectType := range allowedSubjectTypes {
137+
if subjectType.Relation != tuple.Ellipsis {
138+
return warningForPosition(
139+
"arrow-walks-subject-relation",
140+
fmt.Sprintf(
141+
"Arrow `%s->%s` under permission %q references relation %q that has relation %q on subject %q: *the subject relation will be ignored for the arrow*",
142+
ttu.Tupleset.Relation,
143+
ttu.ComputedUserset.Relation,
144+
parentRelation.Name,
145+
ttu.Tupleset.Relation,
146+
subjectType.Relation,
147+
subjectType.Namespace,
148+
),
149+
sourcePosition,
150+
), nil
151+
}
152+
}
153+
154+
return nil, nil
155+
},
156+
}
157+
158+
var lintArrowReferencingRelation = ttuCheck{
159+
"arrow-references-relation",
160+
func(
161+
ctx context.Context,
162+
ttu *corev1.TupleToUserset,
163+
sourcePosition *corev1.SourcePosition,
164+
ts *typesystem.TypeSystem,
165+
) (*devinterface.DeveloperWarning, error) {
166+
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
167+
168+
referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
168169
if !ok {
169-
continue
170+
return nil, nil
170171
}
171172

172-
if !nts.IsPermission(targetRelation.Name) {
173-
return warningForPosition(
174-
fmt.Sprintf(
175-
"Arrow `%s->%s` under permission %q references relation %q on definition %q; it is recommended to point to a permission",
176-
ttu.Tupleset.Relation,
177-
ttu.ComputedUserset.Relation,
178-
parentRelation.Name,
179-
targetRelation.Name,
180-
subjectType.Namespace,
181-
),
182-
sourcePosition,
183-
), nil
173+
// For each subject type of the referenced relation, check if the referenced permission
174+
// is, in fact, a relation.
175+
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
176+
if err != nil {
177+
return nil, err
184178
}
185-
}
186179

187-
return nil, nil
180+
for _, subjectType := range allowedSubjectTypes {
181+
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
182+
if err != nil {
183+
return nil, err
184+
}
185+
186+
targetRelation, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
187+
if !ok {
188+
continue
189+
}
190+
191+
if !nts.IsPermission(targetRelation.Name) {
192+
return warningForPosition(
193+
"arrow-references-relation",
194+
fmt.Sprintf(
195+
"Arrow `%s->%s` under permission %q references relation %q on definition %q; it is recommended to point to a permission",
196+
ttu.Tupleset.Relation,
197+
ttu.ComputedUserset.Relation,
198+
parentRelation.Name,
199+
targetRelation.Name,
200+
subjectType.Namespace,
201+
),
202+
sourcePosition,
203+
), nil
204+
}
205+
}
206+
207+
return nil, nil
208+
},
188209
}

0 commit comments

Comments
 (0)