Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ability to toggle off specific warnings via magic comments #1984

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Add ability to toggle off specific warnings via magic comments #1984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/lsp/lsp_test.go
View file
Open in desktop
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
Open in desktop
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
Loading