Fix positioning on the warnings

authzed · Apr 30, 2024 · ece7ea6 · ece7ea6
1 parent 1c24c8c
commit ece7ea6
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 20 deletions.
diff --git a/pkg/development/warningdefs.go b/pkg/development/warningdefs.go
@@ -37,14 +37,15 @@ var lintRelationReferencesParentType = func(
 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 warningForMetadata(
+		return warningForPosition(
 			fmt.Sprintf("Permission %q references itself, which will cause an error to be raised due to infinite recursion", permName),
-			parentRelation,
+			sourcePosition,
 		), nil
 	}
 
@@ -54,6 +55,7 @@ var lintPermissionReferencingItself = func(
 var lintArrowReferencingUnreachable = func(
 	ctx context.Context,
 	ttu *corev1.TupleToUserset,
+	sourcePosition *corev1.SourcePosition,
 	ts *typesystem.TypeSystem,
 ) (*devinterface.DeveloperWarning, error) {
 	parentRelation := ctx.Value(relationKey).(*corev1.Relation)
@@ -82,7 +84,7 @@ var lintArrowReferencingUnreachable = func(
 	}
 
 	if !wasFound {
-		return warningForMetadata(
+		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,
@@ -91,7 +93,7 @@ var lintArrowReferencingUnreachable = func(
 				ttu.ComputedUserset.Relation,
 				ttu.Tupleset.Relation,
 			),
-			parentRelation,
+			sourcePosition,
 		), nil
 	}
 
@@ -101,6 +103,7 @@ var lintArrowReferencingUnreachable = func(
 var lintArrowOverSubRelation = func(
 	ctx context.Context,
 	ttu *corev1.TupleToUserset,
+	sourcePosition *corev1.SourcePosition,
 	ts *typesystem.TypeSystem,
 ) (*devinterface.DeveloperWarning, error) {
 	parentRelation := ctx.Value(relationKey).(*corev1.Relation)
@@ -117,7 +120,7 @@ var lintArrowOverSubRelation = func(
 
 	for _, subjectType := range allowedSubjectTypes {
 		if subjectType.Relation != tuple.Ellipsis {
-			return warningForMetadata(
+			return warningForPosition(
 				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,
@@ -127,7 +130,7 @@ var lintArrowOverSubRelation = func(
 					subjectType.Relation,
 					subjectType.Namespace,
 				),
-				parentRelation,
+				sourcePosition,
 			), nil
 		}
 	}
@@ -138,6 +141,7 @@ var lintArrowOverSubRelation = func(
 var lintArrowReferencingRelation = func(
 	ctx context.Context,
 	ttu *corev1.TupleToUserset,
+	sourcePosition *corev1.SourcePosition,
 	ts *typesystem.TypeSystem,
 ) (*devinterface.DeveloperWarning, error) {
 	parentRelation := ctx.Value(relationKey).(*corev1.Relation)
@@ -166,7 +170,7 @@ var lintArrowReferencingRelation = func(
 		}
 
 		if !nts.IsPermission(targetRelation.Name) {
-			return warningForMetadata(
+			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,
@@ -175,7 +179,7 @@ var lintArrowReferencingRelation = func(
 					targetRelation.Name,
 					subjectType.Namespace,
 				),
-				parentRelation,
+				sourcePosition,
 			), nil
 		}
 	}

diff --git a/pkg/development/warnings.go b/pkg/development/warnings.go
@@ -25,14 +25,18 @@ var allChecks = checkers{
 }
 
 func warningForMetadata(message string, metadata namespace.WithSourcePosition) *devinterface.DeveloperWarning {
-	if metadata.GetSourcePosition() == nil {
+	return warningForPosition(message, metadata.GetSourcePosition())
+}
+
+func warningForPosition(message string, sourcePosition *corev1.SourcePosition) *devinterface.DeveloperWarning {
+	if sourcePosition == nil {
 		return &devinterface.DeveloperWarning{
 			Message: message,
 		}
 	}
 
-	lineNumber := metadata.GetSourcePosition().ZeroIndexedLineNumber + 1
-	columnNumber := metadata.GetSourcePosition().ZeroIndexedColumnPosition + 1
+	lineNumber := sourcePosition.ZeroIndexedLineNumber + 1
+	columnNumber := sourcePosition.ZeroIndexedColumnPosition + 1
 
 	return &devinterface.DeveloperWarning{
 		Message: message,
@@ -96,8 +100,8 @@ func addDefinitionWarnings(ctx context.Context, def *corev1.NamespaceDefinition,
 
 type (
 	relationChecker        func(ctx context.Context, relation *corev1.Relation, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
-	computedUsersetChecker func(ctx context.Context, computedUserset *corev1.ComputedUserset, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
-	ttuChecker             func(ctx context.Context, ttu *corev1.TupleToUserset, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
+	computedUsersetChecker func(ctx context.Context, computedUserset *corev1.ComputedUserset, sourcePosition *corev1.SourcePosition, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
+	ttuChecker             func(ctx context.Context, ttu *corev1.TupleToUserset, sourcePosition *corev1.SourcePosition, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
 )
 
 type checkers struct {
@@ -135,7 +139,7 @@ func walkUsersetOperations(ctx context.Context, ops []*corev1.SetOperation_Child
 
 		case *corev1.SetOperation_Child_ComputedUserset:
 			for _, checker := range checkers.computedUsersetCheckers {
-				checkerWarning, err := checker(ctx, t.ComputedUserset, ts)
+				checkerWarning, err := checker(ctx, t.ComputedUserset, op.SourcePosition, ts)
 				if err != nil {
 					return nil, err
 				}
@@ -155,7 +159,7 @@ func walkUsersetOperations(ctx context.Context, ops []*corev1.SetOperation_Child
 
 		case *corev1.SetOperation_Child_TupleToUserset:
 			for _, checker := range checkers.ttuCheckers {
-				checkerWarning, err := checker(ctx, t.TupleToUserset, ts)
+				checkerWarning, err := checker(ctx, t.TupleToUserset, op.SourcePosition, ts)
 				if err != nil {
 					return nil, err
 				}

diff --git a/pkg/development/warnings_test.go b/pkg/development/warnings_test.go
@@ -38,7 +38,7 @@ func TestWarnings(t *testing.T) {
 			expectedWarning: &developerv1.DeveloperWarning{
 				Message: "Permission \"view\" references itself, which will cause an error to be raised due to infinite recursion",
 				Line:    2,
-				Column:  5,
+				Column:  23,
 			},
 		},
 		{
@@ -51,7 +51,7 @@ func TestWarnings(t *testing.T) {
 			expectedWarning: &developerv1.DeveloperWarning{
 				Message: "Permission \"view\" references itself, which will cause an error to be raised due to infinite recursion",
 				Line:    4,
-				Column:  5,
+				Column:  42,
 			},
 		},
 		{
@@ -70,7 +70,7 @@ func TestWarnings(t *testing.T) {
 			expectedWarning: &developerv1.DeveloperWarning{
 				Message: "Arrow `group->member` under permission \"view\" references relation \"member\" on definition \"group\"; it is recommended to point to a permission",
 				Line:    9,
-				Column:  5,
+				Column:  23,
 			},
 		},
 		{
@@ -88,7 +88,7 @@ func TestWarnings(t *testing.T) {
 			expectedWarning: &developerv1.DeveloperWarning{
 				Message: "Arrow `group->member` under permission \"view\" references relation/permission \"member\" that does not exist on any subject types of relation \"group\"",
 				Line:    8,
-				Column:  5,
+				Column:  23,
 			},
 		},
 		{
@@ -108,7 +108,7 @@ func TestWarnings(t *testing.T) {
 			expectedWarning: &developerv1.DeveloperWarning{
 				Message: "Arrow `parent_group->member` under permission \"view\" references relation \"parent_group\" that has relation \"member\" on subject \"group\": *the subject relation will be ignored for the arrow*",
 				Line:    10,
-				Column:  5,
+				Column:  23,
 			},
 		},
 		{