Skip to content

Commit

Permalink
Merge pull request #1984 from josephschorr/ignore-warnings
Browse files Browse the repository at this point in the history 
Add ability to toggle off specific warnings via magic comments
  • Loading branch information
@josephschorr
josephschorr committed Jul 11, 2024
2 parents 5ed2d0f + 34b6e21 commit da17bbf
Show file tree
Hide file tree
Showing 4 changed files with 279 additions and 166 deletions.
2 changes: 1 addition & 1 deletion internal/lsp/lsp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestDocumentWarningDiagnostics(t *testing.T) {
require.Equal(t, "full", resp.Kind)
require.Len(t, resp.Items, 1)
require.Equal(t, lsp.DiagnosticSeverity(lsp.Warning), resp.Items[0].Severity)
require.Equal(t, `Permission "view_resource" references parent type "resource" in its name; it is recommended to drop the suffix`, resp.Items[0].Message)
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)
require.Equal(t, lsp.Range{
Start: lsp.Position{Line: 5, Character: 3},
End: lsp.Position{Line: 5, Character: 3},
Expand Down
289 changes: 155 additions & 134 deletions pkg/development/warningdefs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,178 +11,199 @@ import (
"github.com/authzed/spicedb/pkg/typesystem"
)

var lintRelationReferencesParentType = func(
ctx context.Context,
relation *corev1.Relation,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentDef := ts.Namespace()
if strings.HasSuffix(relation.Name, parentDef.Name) {
if ts.IsPermission(relation.Name) {
var lintRelationReferencesParentType = relationCheck{
"relation-name-references-parent",
func(
ctx context.Context,
relation *corev1.Relation,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentDef := ts.Namespace()
if strings.HasSuffix(relation.Name, parentDef.Name) {
if ts.IsPermission(relation.Name) {
return warningForMetadata(
"relation-name-references-parent",
fmt.Sprintf("Permission %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
relation,
), nil
}

return warningForMetadata(
fmt.Sprintf("Permission %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
"relation-name-references-parent",
fmt.Sprintf("Relation %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
relation,
), nil
}

return warningForMetadata(
fmt.Sprintf("Relation %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
relation,
), nil
}

return nil, nil
}

var lintPermissionReferencingItself = func(
ctx context.Context,
computedUserset *corev1.ComputedUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
permName := parentRelation.Name
if computedUserset.Relation == permName {
return warningForPosition(
fmt.Sprintf("Permission %q references itself, which will cause an error to be raised due to infinite recursion", permName),
sourcePosition,
), nil
}

return nil, nil
return nil, nil
},
}

var lintArrowReferencingUnreachable = func(
ctx context.Context,
ttu *corev1.TupleToUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
var lintPermissionReferencingItself = computedUsersetCheck{
"permission-references-itself",
func(
ctx context.Context,
computedUserset *corev1.ComputedUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
permName := parentRelation.Name
if computedUserset.Relation == permName {
return warningForPosition(
"permission-references-itself",
fmt.Sprintf("Permission %q references itself, which will cause an error to be raised due to infinite recursion", permName),
sourcePosition,
), nil
}

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
if !ok {
return nil, nil
}
},
}

allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
if err != nil {
return nil, err
}
var lintArrowReferencingUnreachable = ttuCheck{
"arrow-references-unreachable-relation",
func(
ctx context.Context,
ttu *corev1.TupleToUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
if !ok {
return nil, nil
}

wasFound := false
for _, subjectType := range allowedSubjectTypes {
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
if err != nil {
return nil, err
}

_, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
if ok {
wasFound = true
wasFound := false
for _, subjectType := range allowedSubjectTypes {
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
if err != nil {
return nil, err
}

_, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
if ok {
wasFound = true
}
}
}

if !wasFound {
return warningForPosition(
fmt.Sprintf(
"Arrow `%s->%s` under permission %q references relation/permission %q that does not exist on any subject types of relation %q",
ttu.Tupleset.Relation,
ttu.ComputedUserset.Relation,
parentRelation.Name,
ttu.ComputedUserset.Relation,
ttu.Tupleset.Relation,
),
sourcePosition,
), nil
}

return nil, nil
}

var lintArrowOverSubRelation = func(
ctx context.Context,
ttu *corev1.TupleToUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
if !ok {
return nil, nil
}

allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
if err != nil {
return nil, err
}

for _, subjectType := range allowedSubjectTypes {
if subjectType.Relation != tuple.Ellipsis {
if !wasFound {
return warningForPosition(
"arrow-references-unreachable-relation",
fmt.Sprintf(
"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*",
"Arrow `%s->%s` under permission %q references relation/permission %q that does not exist on any subject types of relation %q",
ttu.Tupleset.Relation,
ttu.ComputedUserset.Relation,
parentRelation.Name,
ttu.ComputedUserset.Relation,
ttu.Tupleset.Relation,
subjectType.Relation,
subjectType.Namespace,
),
sourcePosition,
), nil
}
}

return nil, nil
}

var lintArrowReferencingRelation = func(
ctx context.Context,
ttu *corev1.TupleToUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
if !ok {
return nil, nil
}
},
}

// For each subject type of the referenced relation, check if the referenced permission
// is, in fact, a relation.
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
if err != nil {
return nil, err
}
var lintArrowOverSubRelation = ttuCheck{
"arrow-walks-subject-relation",
func(
ctx context.Context,
ttu *corev1.TupleToUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
if !ok {
return nil, nil
}

for _, subjectType := range allowedSubjectTypes {
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
if err != nil {
return nil, err
}

targetRelation, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
for _, subjectType := range allowedSubjectTypes {
if subjectType.Relation != tuple.Ellipsis {
return warningForPosition(
"arrow-walks-subject-relation",
fmt.Sprintf(
"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*",
ttu.Tupleset.Relation,
ttu.ComputedUserset.Relation,
parentRelation.Name,
ttu.Tupleset.Relation,
subjectType.Relation,
subjectType.Namespace,
),
sourcePosition,
), nil
}
}

return nil, nil
},
}

var lintArrowReferencingRelation = ttuCheck{
"arrow-references-relation",
func(
ctx context.Context,
ttu *corev1.TupleToUserset,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
if !ok {
continue
return nil, nil
}

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

return nil, nil
for _, subjectType := range allowedSubjectTypes {
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
if err != nil {
return nil, err
}

targetRelation, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
if !ok {
continue
}

if !nts.IsPermission(targetRelation.Name) {
return warningForPosition(
"arrow-references-relation",
fmt.Sprintf(
"Arrow `%s->%s` under permission %q references relation %q on definition %q; it is recommended to point to a permission",
ttu.Tupleset.Relation,
ttu.ComputedUserset.Relation,
parentRelation.Name,
targetRelation.Name,
subjectType.Namespace,
),
sourcePosition,
), nil
}
}

return nil, nil
},
}
Loading

0 comments on commit da17bbf

Please sign in to comment.