test(linter): address self-review findings

- Use fmt.Sprintf for entity IDs (was fragile rune arithmetic)
- Add RuleID assertion to WebServiceCall test
- Add NOTE comments for reader-dependent rules (MPR008, SEC002, SEC003)

Author: retran

mdl/executor/cmd_odata.go

@@ -1496,38 +1496,38 @@ func fetchODataMetadata(metadataUrl string) (metadata string, hash string, err e
 		return "", "", nil
 	}
 
-  var body []byte
-
-  // At this point, metadataUrl is already normalized by NormalizeURL() in createODataClient:
-  // - Relative paths have been converted to absolute file:// URLs
-  // - HTTP(S) URLs are unchanged
-  // So we only need to distinguish file:// vs HTTP(S)
-
-  filePath := pathutil.PathFromURL(metadataUrl)
-  if filePath != "" {
-      // Local file - read directly (path is already absolute)
-      body, err = os.ReadFile(filePath)
-      if err != nil {
-          return "", "", mdlerrors.NewBackend(fmt.Sprintf("read local metadata file %s", filePath), err)
-      }
-  } else {
-      // HTTP(S) fetch
-      client := &http.Client{Timeout: 30 * time.Second}
-      resp, err := client.Get(metadataUrl)
-      if err != nil {
-          return "", "", mdlerrors.NewBackend(fmt.Sprintf("fetch $metadata from %s", metadataUrl), err)
-      }
-      defer resp.Body.Close()
-
-      if resp.StatusCode != http.StatusOK {
-          return "", "", mdlerrors.NewValidationf("$metadata fetch returned HTTP %d from %s", resp.StatusCode, metadataUrl)
-      }
-
-      body, err = io.ReadAll(resp.Body)
-      if err != nil {
-          return "", "", mdlerrors.NewBackend("read $metadata response", err)
-      }
-  }
+	var body []byte
+
+	// At this point, metadataUrl is already normalized by NormalizeURL() in createODataClient:
+	// - Relative paths have been converted to absolute file:// URLs
+	// - HTTP(S) URLs are unchanged
+	// So we only need to distinguish file:// vs HTTP(S)
+
+	filePath := pathutil.PathFromURL(metadataUrl)
+	if filePath != "" {
+		// Local file - read directly (path is already absolute)
+		body, err = os.ReadFile(filePath)
+		if err != nil {
+			return "", "", mdlerrors.NewBackend(fmt.Sprintf("read local metadata file %s", filePath), err)
+		}
+	} else {
+		// HTTP(S) fetch
+		client := &http.Client{Timeout: 30 * time.Second}
+		resp, err := client.Get(metadataUrl)
+		if err != nil {
+			return "", "", mdlerrors.NewBackend(fmt.Sprintf("fetch $metadata from %s", metadataUrl), err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusOK {
+			return "", "", mdlerrors.NewValidationf("$metadata fetch returned HTTP %d from %s", resp.StatusCode, metadataUrl)
+		}
+
+		body, err = io.ReadAll(resp.Body)
+		if err != nil {
+			return "", "", mdlerrors.NewBackend("read $metadata response", err)
+		}
+	}
 
 	// Hash calculation (same for both HTTP and local file)
 	metadata = string(body)

mdl/linter/rules/conv_error_handling_test.go

@@ -105,6 +105,9 @@ func TestFindUnhandledCalls_WebServiceCall(t *testing.T) {
 	if len(violations) != 1 {
 		t.Fatalf("expected 1 violation for WS call, got %d", len(violations))
 	}
+	if violations[0].RuleID != "CONV013" {
+		t.Errorf("expected CONV013, got %s", violations[0].RuleID)
+	}
 }
 
 func TestFindUnhandledCalls_NonExternalAction(t *testing.T) {

mdl/linter/rules/domain_size_test.go

@@ -4,6 +4,7 @@ package rules
 
 import (
 	"database/sql"
+	"fmt"
 	"testing"
 
 	"github.com/mendixlabs/mxcli/mdl/linter"
@@ -51,8 +52,8 @@ func TestDomainModelSizeRule_NoViolation(t *testing.T) {
 	var entities [][]any
 	for i := 0; i < 10; i++ {
 		entities = append(entities, []any{
-			"id" + string(rune('0'+i)), "Entity" + string(rune('0'+i)),
-			"MyModule.Entity" + string(rune('0'+i)), "MyModule", "",
+			fmt.Sprintf("id%d", i), fmt.Sprintf("Entity%d", i),
+			fmt.Sprintf("MyModule.Entity%d", i), "MyModule", "",
 			"PERSISTENT", "", "", 5, 1, 0, 0, 0,
 		})
 	}
@@ -73,8 +74,8 @@ func TestDomainModelSizeRule_ExceedsThreshold(t *testing.T) {
 	var entities [][]any
 	for i := 0; i < 20; i++ {
 		entities = append(entities, []any{
-			"id" + string(rune('A'+i)), "Entity" + string(rune('A'+i)),
-			"BigModule.Entity" + string(rune('A'+i)), "BigModule", "",
+			fmt.Sprintf("id%d", i), fmt.Sprintf("Entity%d", i),
+			fmt.Sprintf("BigModule.Entity%d", i), "BigModule", "",
 			"PERSISTENT", "", "", 3, 1, 0, 0, 0,
 		})
 	}
@@ -98,8 +99,8 @@ func TestDomainModelSizeRule_NonPersistentIgnored(t *testing.T) {
 	var entities [][]any
 	for i := 0; i < 20; i++ {
 		entities = append(entities, []any{
-			"id" + string(rune('A'+i)), "Entity" + string(rune('A'+i)),
-			"MyModule.Entity" + string(rune('A'+i)), "MyModule", "",
+			fmt.Sprintf("id%d", i), fmt.Sprintf("Entity%d", i),
+			fmt.Sprintf("MyModule.Entity%d", i), "MyModule", "",
 			"NON_PERSISTENT", "", "", 3, 0, 0, 0, 0,
 		})
 	}

mdl/linter/rules/mpr008_overlapping_activities_test.go

@@ -8,6 +8,11 @@ import (
 	"github.com/mendixlabs/mxcli/mdl/linter"
 )
 
+// NOTE: The full Check() logic requires ctx.Reader().GetMicroflow() to read microflow
+// positions from a real MPR file. The overlap detection algorithm (collect positions →
+// pairwise distance check) is inline in Check() and cannot be unit-tested without
+// building a mock mpr.Reader. Covered by integration tests (roundtrip_*.go).
+
 func TestOverlappingActivitiesRule_NilReader(t *testing.T) {
 	r := NewOverlappingActivitiesRule()
 	ctx := linter.NewLintContextFromDB(nil)

mdl/linter/rules/security_test.go

@@ -88,8 +88,9 @@ func TestNoEntityAccessRulesRule_Metadata(t *testing.T) {
 	}
 }
 
-// SEC002 and SEC003 require ctx.Reader() which returns *mpr.Reader.
-// These rules are tested via metadata + nil-reader guard only.
+// NOTE: SEC002 and SEC003 require ctx.Reader() → *mpr.Reader to call GetProjectSecurity().
+// Without a real MPR file, we can only test the nil-reader early return and metadata.
+// Full behavioral coverage requires integration tests with a real .mpr project.
 
 func TestWeakPasswordPolicyRule_NilReader(t *testing.T) {
 	db, _ := sql.Open("sqlite", ":memory:")