Merge pull request #84 from mxlint/feature/improve-noqa

Feature/improve noqa with --ignore-noqa and rule level skipping

Author: xiwenc

README.md

@@ -89,9 +89,62 @@ PASS (0.00158s) modelsource/MyFirstModule/DomainModels$DomainModel.yaml
 ![Mendix Lint report](./resources/lint-xunit-report.png)
 Lint Mendix Yaml files. This tool checks for common mistakes and enforces best practices. It uses OPA as policy engine. Therefore policies must be written in the powerful Rego language. Please refer to [Rego language reference](https://www.openpolicyagent.org/docs/latest/policy-reference/) for more information on the syntax and semantics.
 
-### NOQA (Ignore document)
+### NOQA (Ignore document or specific rules)
 
-A specific document can be marked as "Skipped" if you have a line in the `documentation` field that starts with either `#noqa` or `# noqa` followed by an optional message (Case in-sensitive). This message will be included as "Skipped" reason in linting results.
+Documents can be marked with noqa directives in the `documentation` field to skip linting. There are two supported formats:
+
+#### Skip all rules (document-level noqa)
+
+A specific document can be marked as "Skipped" for all rules if you have a line in the `documentation` field that starts with either `#noqa` or `# noqa` followed by an optional message (Case in-sensitive). This message will be included as "Skipped" reason in linting results.
+
+Example:
+```yaml
+Documentation: |
+  #noqa This document is excluded from all linting rules
+```
+
+#### Skip specific rules (rule-level noqa)
+
+You can skip specific rules by providing a comma-separated list of rule numbers after a colon. This allows fine-grained control over which rules to ignore.
+
+Example:
+```yaml
+Documentation: |
+  #noqa:001_0002,001_0003 Temporarily skipping these rules due to legacy code
+```
+
+In this example, only rules `001_0002` and `001_0003` will be skipped for this document, while all other rules will still be evaluated.
+
+**Syntax:**
+- `#noqa` or `# noqa` - Skip all rules
+- `#noqa:rule1,rule2,...` or `# noqa:rule1,rule2,...` - Skip specific rules
+- Optional reason can be added after the rule list, separated by a space
+
+**Notes:**
+- The noqa directive is case-insensitive
+- Multiple rule numbers should be separated by commas (no spaces)
+- The entire line after the noqa directive will be recorded as the skip reason
+
+#### Ignore NOQA directives (--ignore-noqa flag)
+
+In some scenarios, you may want to run linting on all documents, including those marked with noqa directives. The `--ignore-noqa` flag allows you to temporarily disable the noqa functionality.
+
+**Usage:**
+```bash
+./mxlint-cli lint --ignore-noqa
+```
+
+When this flag is set:
+- All noqa directives (both document-level and rule-level) are ignored
+- Documents that would normally be skipped are evaluated against all rules
+- The default behavior is `false` (noqa directives are respected)
+
+**Example scenarios:**
+- **Audit mode**: Run a complete check to see all violations, even in documents marked with noqa
+- **CI/CD validation**: Enforce that certain branches (e.g., production) don't rely on noqa suppressions
+- **Cleanup**: Identify which noqa markers can be removed after fixing underlying issues
+
+**Note:** When using `--ignore-noqa`, results are not cached to ensure consistency with normal linting behavior.
 
 ## serve
 

lint/lint.go

@@ -30,7 +30,7 @@ func printTestsuite(ts Testsuite) {
 
 // EvalAllWithResults evaluates all rules and returns the results
 // This is similar to EvalAll but returns the results instead of just printing them
-func EvalAllWithResults(rulesPath string, modelSourcePath string, xunitReport string, jsonFile string) (interface{}, error) {
+func EvalAllWithResults(rulesPath string, modelSourcePath string, xunitReport string, jsonFile string, ignoreNoqa bool) (interface{}, error) {
 	rules, err := ReadRulesMetadata(rulesPath)
 	if err != nil {
 		return nil, err
@@ -54,7 +54,7 @@ func EvalAllWithResults(rulesPath string, modelSourcePath string, xunitReport st
 		go func(index int, r Rule) {
 			defer wg.Done()
 
-			testsuite, err := evalTestsuite(r, modelSourcePath)
+			testsuite, err := evalTestsuite(r, modelSourcePath, ignoreNoqa)
 			if err != nil {
 				errChan <- err
 				return
@@ -138,7 +138,7 @@ func EvalAllWithResults(rulesPath string, modelSourcePath string, xunitReport st
 	return testsuitesContainer, nil
 }
 
-func EvalAll(rulesPath string, modelSourcePath string, xunitReport string, jsonFile string) error {
+func EvalAll(rulesPath string, modelSourcePath string, xunitReport string, jsonFile string, ignoreNoqa bool) error {
 	rules, err := ReadRulesMetadata(rulesPath)
 	if err != nil {
 		return err
@@ -162,7 +162,7 @@ func EvalAll(rulesPath string, modelSourcePath string, xunitReport string, jsonF
 		go func(index int, r Rule) {
 			defer wg.Done()
 
-			testsuite, err := evalTestsuite(r, modelSourcePath)
+			testsuite, err := evalTestsuite(r, modelSourcePath, ignoreNoqa)
 			if err != nil {
 				errChan <- err
 				return
@@ -259,7 +259,7 @@ func countTotalTestcases(testsuites []Testsuite) int {
 	return count
 }
 
-func evalTestsuite(rule Rule, modelSourcePath string) (*Testsuite, error) {
+func evalTestsuite(rule Rule, modelSourcePath string, ignoreNoqa bool) (*Testsuite, error) {
 
 	log.Debugf("evaluating rule %s", rule.Path)
 
@@ -276,30 +276,36 @@ func evalTestsuite(rule Rule, modelSourcePath string) (*Testsuite, error) {
 
 	for _, inputFile := range inputFiles {
 
-		// Try to load from cache first
+		// Try to load from cache first (but skip cache if ignoreNoqa is true)
 		cacheKey, err := createCacheKey(rule.Path, inputFile)
 		if err != nil {
 			log.Debugf("Error creating cache key: %v", err)
-		} else {
+		} else if !ignoreNoqa {
 			cachedTestcase, found := loadCachedTestcase(*cacheKey)
 			if found {
 				testcase = cachedTestcase
 				log.Debugf("Using cached result for %s", inputFile)
 			} else {
 				// Cache miss - evaluate and save to cache
-				testcase, err = evalTestcaseWithCaching(rule, queryString, inputFile, cacheKey)
+				testcase, err = evalTestcaseWithCaching(rule, queryString, inputFile, cacheKey, ignoreNoqa)
 				if err != nil {
 					return nil, err
 				}
 			}
+		} else {
+			// ignoreNoqa is true, skip cache and evaluate directly
+			testcase, err = evalTestcaseWithCaching(rule, queryString, inputFile, cacheKey, ignoreNoqa)
+			if err != nil {
+				return nil, err
+			}
 		}
 
 		// Fallback if cache key creation failed
 		if cacheKey == nil {
 			if rule.Language == LanguageRego {
-				testcase, err = evalTestcase_Rego(rule.Path, queryString, inputFile)
+				testcase, err = evalTestcase_Rego(rule.Path, queryString, inputFile, rule.RuleNumber, ignoreNoqa)
 			} else if rule.Language == LanguageJavascript {
-				testcase, err = evalTestcase_Javascript(rule.Path, inputFile)
+				testcase, err = evalTestcase_Javascript(rule.Path, inputFile, rule.RuleNumber, ignoreNoqa)
 			}
 			if err != nil {
 				return nil, err
@@ -332,24 +338,27 @@ func evalTestsuite(rule Rule, modelSourcePath string) (*Testsuite, error) {
 }
 
 // evalTestcaseWithCaching evaluates a testcase and saves the result to cache
-func evalTestcaseWithCaching(rule Rule, queryString string, inputFile string, cacheKey *CacheKey) (*Testcase, error) {
+func evalTestcaseWithCaching(rule Rule, queryString string, inputFile string, cacheKey *CacheKey, ignoreNoqa bool) (*Testcase, error) {
 	var testcase *Testcase
 	var err error
 
 	if rule.Language == LanguageRego {
-		testcase, err = evalTestcase_Rego(rule.Path, queryString, inputFile)
+		testcase, err = evalTestcase_Rego(rule.Path, queryString, inputFile, rule.RuleNumber, ignoreNoqa)
 	} else if rule.Language == LanguageJavascript {
-		testcase, err = evalTestcase_Javascript(rule.Path, inputFile)
+		testcase, err = evalTestcase_Javascript(rule.Path, inputFile, rule.RuleNumber, ignoreNoqa)
 	}
 
 	if err != nil {
 		return nil, err
 	}
 
-	// Save to cache
-	if cacheErr := saveCachedTestcase(*cacheKey, testcase); cacheErr != nil {
-		log.Debugf("Error saving to cache: %v", cacheErr)
-		// Don't fail the evaluation if cache save fails
+	// Only save to cache when ignoreNoqa is false
+	// When ignoreNoqa is true, the result might differ from the normal behavior
+	if !ignoreNoqa {
+		if cacheErr := saveCachedTestcase(*cacheKey, testcase); cacheErr != nil {
+			log.Debugf("Error saving to cache: %v", cacheErr)
+			// Don't fail the evaluation if cache save fails
+		}
 	}
 
 	return testcase, nil

lint/lint_javascript.go

@@ -10,7 +10,7 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-func evalTestcase_Javascript(rulePath string, inputFilePath string) (*Testcase, error) {
+func evalTestcase_Javascript(rulePath string, inputFilePath string, ruleNumber string, ignoreNoqa bool) (*Testcase, error) {
 	ruleContent, _ := os.ReadFile(rulePath)
 	log.Debugf("js file: \n%s", ruleContent)
 
@@ -34,19 +34,15 @@ func evalTestcase_Javascript(rulePath string, inputFilePath string) (*Testcase,
 		return nil, err
 	}
 
-	// if data["Documentation"] contains #noqa, skip the testcase; Documentation attribute might not exist
+	// Check if this rule should be skipped based on noqa directives
 	if doc, ok := data["Documentation"].(string); ok {
-		lines := strings.Split(doc, "\n")
-		for _, line := range lines {
-			line = strings.TrimSpace(line)
-			lineLower := strings.ToLower(line)
-			if strings.HasPrefix(lineLower, NOQA) || strings.HasPrefix(lineLower, NOQA_ALIAS) {
-				return &Testcase{
-					Name:    inputFilePath,
-					Time:    0,
-					Skipped: &Skipped{Message: line},
-				}, nil
-			}
+		shouldSkip, reason := shouldSkipRule(doc, ruleNumber, ignoreNoqa)
+		if shouldSkip {
+			return &Testcase{
+				Name:    inputFilePath,
+				Time:    0,
+				Skipped: &Skipped{Message: reason},
+			}, nil
 		}
 	}
 

lint/lint_rego.go

@@ -11,7 +11,7 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-func evalTestcase_Rego(rulePath string, queryString string, inputFilePath string) (*Testcase, error) {
+func evalTestcase_Rego(rulePath string, queryString string, inputFilePath string, ruleNumber string, ignoreNoqa bool) (*Testcase, error) {
 	regoFile, _ := os.ReadFile(rulePath)
 	log.Debugf("rego file: \n%s", regoFile)
 
@@ -34,19 +34,15 @@ func evalTestcase_Rego(rulePath string, queryString string, inputFilePath string
 		return nil, err
 	}
 
-	// if data["Documentation"] contains #noqa, skip the testcase; Documentation attribute might not exist
+	// Check if this rule should be skipped based on noqa directives
 	if doc, ok := data["Documentation"].(string); ok {
-		lines := strings.Split(doc, "\n")
-		for _, line := range lines {
-			line = strings.TrimSpace(line)
-			lineLower := strings.ToLower(line)
-			if strings.HasPrefix(lineLower, NOQA) || strings.HasPrefix(lineLower, NOQA_ALIAS) {
-				return &Testcase{
-					Name:    inputFilePath,
-					Time:    0,
-					Skipped: &Skipped{Message: line},
-				}, nil
-			}
+		shouldSkip, reason := shouldSkipRule(doc, ruleNumber, ignoreNoqa)
+		if shouldSkip {
+			return &Testcase{
+				Name:    inputFilePath,
+				Time:    0,
+				Skipped: &Skipped{Message: reason},
+			}, nil
 		}
 	}
 

lint/lint_test.go

@@ -19,7 +19,7 @@ func TestLintSingle(t *testing.T) {
 	// })
 	t.Run("single Rego rule passes", func(t *testing.T) {
 		rule, _ := parseRuleMetadata_Rego("./../resources/rules/001_0003_security_checks.rego")
-		result, err := evalTestsuite(*rule, "./../resources/modelsource-v1")
+		result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false)
 
 		if err != nil {
 			t.Errorf("Failed to evaluate")
@@ -31,7 +31,7 @@ func TestLintSingle(t *testing.T) {
 	})
 	t.Run("single JS rule passes", func(t *testing.T) {
 		rule, _ := parseRuleMetadata_Javascript("./../resources/rules/001_0002_demo_users_disabled.js")
-		result, err := evalTestsuite(*rule, "./../resources/modelsource-v1")
+		result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false)
 
 		if err != nil {
 			t.Errorf("Failed to evaluate")
@@ -45,7 +45,7 @@ func TestLintSingle(t *testing.T) {
 
 func TestLintBundle(t *testing.T) {
 	t.Run("all-rules", func(t *testing.T) {
-		err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", "", "")
+		err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", "", "", false)
 
 		if err != nil {
 			t.Errorf("No failures expected: %v", err)

lint/utils.go

@@ -16,6 +16,94 @@ func SetLogger(logger *logrus.Logger) {
 	log = logger
 }
 
+// parseNoqaDirective parses a noqa directive and returns the list of rules to skip
+// and the reason (if provided).
+// Supports two formats:
+// - "#noqa" or "# noqa" - skips all rules
+// - "#noqa:rule1,rule2" or "# noqa:rule1,rule2 reason" - skips specific rules
+// Returns: (skipAllRules bool, skipRules []string, reason string)
+func parseNoqaDirective(line string) (bool, []string, string) {
+	line = strings.TrimSpace(line)
+	lineLower := strings.ToLower(line)
+	
+	// Check if line starts with #noqa or # noqa
+	if !strings.HasPrefix(lineLower, NOQA) && !strings.HasPrefix(lineLower, NOQA_ALIAS) {
+		return false, nil, ""
+	}
+	
+	// Remove the prefix to get the rest
+	var rest string
+	if strings.HasPrefix(lineLower, NOQA) {
+		rest = strings.TrimSpace(line[len(NOQA):])
+	} else {
+		rest = strings.TrimSpace(line[len(NOQA_ALIAS):])
+	}
+	
+	// If nothing follows, skip all rules
+	if rest == "" {
+		return true, nil, line
+	}
+	
+	// Check if it starts with colon (rule-specific noqa)
+	if strings.HasPrefix(rest, ":") {
+		rest = strings.TrimPrefix(rest, ":")
+		
+		// Split by space to separate rules from reason
+		parts := strings.SplitN(rest, " ", 2)
+		rulesStr := strings.TrimSpace(parts[0])
+		reason := line // Use full line as reason
+		
+		// Split rules by comma
+		rules := strings.Split(rulesStr, ",")
+		skipRules := make([]string, 0, len(rules))
+		for _, rule := range rules {
+			rule = strings.TrimSpace(rule)
+			if rule != "" {
+				skipRules = append(skipRules, rule)
+			}
+		}
+		
+		if len(skipRules) > 0 {
+			return false, skipRules, reason
+		}
+	}
+	
+	// Default: skip all rules with the line as reason
+	return true, nil, line
+}
+
+// shouldSkipRule checks if a specific rule should be skipped based on noqa directives
+// in the documentation field
+func shouldSkipRule(documentation string, ruleNumber string, ignoreNoqa bool) (bool, string) {
+	// If ignoreNoqa is true, never skip rules based on noqa directives
+	if ignoreNoqa {
+		return false, ""
+	}
+	
+	if documentation == "" {
+		return false, ""
+	}
+	
+	lines := strings.Split(documentation, "\n")
+	for _, line := range lines {
+		skipAll, skipRules, reason := parseNoqaDirective(line)
+		
+		// If skipAll is true, skip this rule
+		if skipAll {
+			return true, reason
+		}
+		
+		// Check if this specific rule is in the skip list
+		for _, skipRule := range skipRules {
+			if skipRule == ruleNumber {
+				return true, reason
+			}
+		}
+	}
+	
+	return false, ""
+}
+
 func expandPaths(pattern string, workingDirectory string) ([]string, error) {
 	// backwards compatible with old filepath.glob(...)
 	if !strings.HasPrefix(pattern, ".*") {