From 3e65c36d888a87aee836a4cb2144be967b56d79b Mon Sep 17 00:00:00 2001 From: Xiwen Cheng Date: Wed, 8 Jul 2026 00:14:52 +0200 Subject: [PATCH] Support diff linting --- lint/diff_filter.go | 27 +++++++++ lint/diff_filter_test.go | 63 +++++++++++++++++++++ lint/git.go | 116 +++++++++++++++++++++++++++++++++++++++ lint/git_test.go | 66 ++++++++++++++++++++++ lint/lint.go | 11 ++-- lint/lint_test.go | 30 +++++----- main.go | 35 ++++++++++++ serve/serve.go | 1 + 8 files changed, 329 insertions(+), 20 deletions(-) create mode 100644 lint/diff_filter.go create mode 100644 lint/diff_filter_test.go create mode 100644 lint/git.go create mode 100644 lint/git_test.go diff --git a/lint/diff_filter.go b/lint/diff_filter.go new file mode 100644 index 0000000..0038406 --- /dev/null +++ b/lint/diff_filter.go @@ -0,0 +1,27 @@ +package lint + +func normalizeChangedFilesSet(changedFiles []string) map[string]struct{} { + if changedFiles == nil { + return nil + } + + set := make(map[string]struct{}, len(changedFiles)) + for _, file := range changedFiles { + set[cleanPath(file)] = struct{}{} + } + return set +} + +func filterInputFiles(inputFiles []string, changedFiles map[string]struct{}) []string { + if changedFiles == nil { + return inputFiles + } + + filtered := make([]string, 0, len(inputFiles)) + for _, inputFile := range inputFiles { + if _, ok := changedFiles[cleanPath(inputFile)]; ok { + filtered = append(filtered, inputFile) + } + } + return filtered +} diff --git a/lint/diff_filter_test.go b/lint/diff_filter_test.go new file mode 100644 index 0000000..36f4b15 --- /dev/null +++ b/lint/diff_filter_test.go @@ -0,0 +1,63 @@ +package lint + +import ( + "os" + "path/filepath" + "testing" +) + +func TestFilterFilesUnderDirectory(t *testing.T) { + tempDir := t.TempDir() + modelDir := filepath.Join(tempDir, "modelsource") + nestedDir := filepath.Join(modelDir, "Module2") + if err := os.MkdirAll(nestedDir, 0755); err != nil { + t.Fatalf("failed to create directories: %v", err) + } + + inside := filepath.Join(nestedDir, "DomainModels$DomainModel.yaml") + outside := filepath.Join(tempDir, "mxlint.yaml") + for _, path := range []string{inside, outside} { + if err := os.WriteFile(path, []byte("name: test\n"), 0644); err != nil { + t.Fatalf("failed to write file %q: %v", path, err) + } + } + + filtered, err := FilterFilesUnderDirectory([]string{inside, outside}, modelDir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(filtered) != 1 { + t.Fatalf("expected 1 file, got %d (%v)", len(filtered), filtered) + } + if filtered[0] != cleanPath(inside) { + t.Fatalf("expected %q, got %q", cleanPath(inside), filtered[0]) + } +} + +func TestFilterInputFiles(t *testing.T) { + tempDir := t.TempDir() + changed := filepath.Join(tempDir, "changed.yaml") + unchanged := filepath.Join(tempDir, "unchanged.yaml") + for _, path := range []string{changed, unchanged} { + if err := os.WriteFile(path, []byte("name: test\n"), 0644); err != nil { + t.Fatalf("failed to write file %q: %v", path, err) + } + } + + changedSet := normalizeChangedFilesSet([]string{changed}) + filtered := filterInputFiles([]string{changed, unchanged}, changedSet) + if len(filtered) != 1 { + t.Fatalf("expected 1 file, got %d (%v)", len(filtered), filtered) + } + if filtered[0] != changed { + t.Fatalf("expected %q, got %q", changed, filtered[0]) + } +} + +func TestFilterInputFilesNilSetReturnsAll(t *testing.T) { + inputFiles := []string{"a.yaml", "b.yaml"} + filtered := filterInputFiles(inputFiles, nil) + if len(filtered) != len(inputFiles) { + t.Fatalf("expected all files to be returned, got %v", filtered) + } +} diff --git a/lint/git.go b/lint/git.go new file mode 100644 index 0000000..3c1b0e5 --- /dev/null +++ b/lint/git.go @@ -0,0 +1,116 @@ +package lint + +import ( + "errors" + "fmt" + "os/exec" + "path/filepath" + "strings" +) + +var ErrNotGitRepository = errors.New("not a git repository") + +// IsGitRepository reports whether dir is inside a git work tree. +func IsGitRepository(dir string) (bool, error) { + cmd := exec.Command("git", "rev-parse", "--is-inside-work-tree") + cmd.Dir = dir + output, err := cmd.Output() + if err != nil { + if isGitCommandNotFound(err) { + return false, fmt.Errorf("git is not installed or not available in PATH") + } + return false, nil + } + return strings.TrimSpace(string(output)) == "true", nil +} + +// GitUnstagedChangedFiles returns absolute paths of files with unstaged changes. +func GitUnstagedChangedFiles(dir string) ([]string, error) { + isRepo, err := IsGitRepository(dir) + if err != nil { + return nil, err + } + if !isRepo { + return nil, ErrNotGitRepository + } + + gitRoot, err := gitTopLevel(dir) + if err != nil { + return nil, err + } + + cmd := exec.Command("git", "diff", "--name-only", "--diff-filter=ACMR") + cmd.Dir = dir + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("failed to list unstaged changes: %w", err) + } + + lines := strings.Split(strings.TrimSpace(string(output)), "\n") + changedFiles := make([]string, 0, len(lines)) + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + changedFiles = append(changedFiles, cleanPath(filepath.Join(gitRoot, line))) + } + return changedFiles, nil +} + +// FilterFilesUnderDirectory keeps only files located under directory. +func FilterFilesUnderDirectory(files []string, directory string) ([]string, error) { + absDir, err := filepath.Abs(directory) + if err != nil { + return nil, fmt.Errorf("failed to resolve directory %q: %w", directory, err) + } + absDir = cleanPath(absDir) + + filtered := make([]string, 0, len(files)) + for _, file := range files { + absFile, err := filepath.Abs(file) + if err != nil { + continue + } + absFile = cleanPath(absFile) + rel, err := filepath.Rel(absDir, absFile) + if err != nil || strings.HasPrefix(rel, "..") { + continue + } + filtered = append(filtered, absFile) + } + return filtered, nil +} + +func gitTopLevel(dir string) (string, error) { + cmd := exec.Command("git", "rev-parse", "--show-toplevel") + cmd.Dir = dir + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("failed to resolve git root: %w", err) + } + return strings.TrimSpace(string(output)), nil +} + +func isGitCommandNotFound(err error) bool { + if err == nil { + return false + } + if errors.Is(err, exec.ErrNotFound) { + return true + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + return exitErr.ExitCode() == 127 + } + return false +} + +func cleanPath(path string) string { + cleaned := filepath.Clean(path) + resolved, err := filepath.EvalSymlinks(cleaned) + if err != nil { + return cleaned + } + return resolved +} diff --git a/lint/git_test.go b/lint/git_test.go new file mode 100644 index 0000000..84cb499 --- /dev/null +++ b/lint/git_test.go @@ -0,0 +1,66 @@ +package lint + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +func TestGitUnstagedChangedFilesRequiresRepository(t *testing.T) { + tempDir := t.TempDir() + _, err := GitUnstagedChangedFiles(tempDir) + if err == nil { + t.Fatal("expected error for non-git directory") + } + if err != ErrNotGitRepository { + t.Fatalf("expected ErrNotGitRepository, got %v", err) + } +} + +func TestGitUnstagedChangedFilesDetectsUnstagedModelDocument(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git is not available") + } + + tempDir := t.TempDir() + runGit(t, tempDir, "init") + runGit(t, tempDir, "config", "user.email", "mxlint@test.local") + runGit(t, tempDir, "config", "user.name", "mxlint test") + + modelDir := filepath.Join(tempDir, "modelsource") + if err := os.MkdirAll(modelDir, 0755); err != nil { + t.Fatalf("failed to create model directory: %v", err) + } + + docPath := filepath.Join(modelDir, "Security$ProjectSecurity.yaml") + if err := os.WriteFile(docPath, []byte("name: initial\n"), 0644); err != nil { + t.Fatalf("failed to write model document: %v", err) + } + runGit(t, tempDir, "add", "modelsource/Security$ProjectSecurity.yaml") + runGit(t, tempDir, "commit", "-m", "initial") + + if err := os.WriteFile(docPath, []byte("name: changed\n"), 0644); err != nil { + t.Fatalf("failed to update model document: %v", err) + } + + changedFiles, err := GitUnstagedChangedFiles(tempDir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(changedFiles) != 1 { + t.Fatalf("expected 1 changed file, got %d (%v)", len(changedFiles), changedFiles) + } + if changedFiles[0] != cleanPath(docPath) { + t.Fatalf("expected %q, got %q", cleanPath(docPath), changedFiles[0]) + } +} + +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v failed: %v (%s)", args, err, string(output)) + } +} diff --git a/lint/lint.go b/lint/lint.go index cbeeffd..9cdbe6b 100644 --- a/lint/lint.go +++ b/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, ignoreNoqa bool, useCache bool) (interface{}, error) { +func EvalAllWithResults(rulesPath string, modelSourcePath string, xunitReport string, jsonFile string, ignoreNoqa bool, useCache bool, changedFiles []string) (interface{}, error) { rules, err := ReadRulesMetadata(rulesPath) if err != nil { return nil, err @@ -59,7 +59,7 @@ func EvalAllWithResults(rulesPath string, modelSourcePath string, xunitReport st defer wg.Done() defer func() { <-sem }() - testsuite, err := evalTestsuite(r, modelSourcePath, ignoreNoqa, useCache) + testsuite, err := evalTestsuite(r, modelSourcePath, ignoreNoqa, useCache, changedFiles) if err != nil { errChan <- err return @@ -143,7 +143,7 @@ func EvalAllWithResults(rulesPath string, modelSourcePath string, xunitReport st return testsuitesContainer, nil } -func EvalAll(rulesPath string, modelSourcePath string, xunitReport string, jsonFile string, ignoreNoqa bool, useCache bool) error { +func EvalAll(rulesPath string, modelSourcePath string, xunitReport string, jsonFile string, ignoreNoqa bool, useCache bool, changedFiles []string) error { rules, err := ReadRulesMetadata(rulesPath) if err != nil { return err @@ -172,7 +172,7 @@ func EvalAll(rulesPath string, modelSourcePath string, xunitReport string, jsonF defer wg.Done() defer func() { <-sem }() - testsuite, err := evalTestsuite(r, modelSourcePath, ignoreNoqa, useCache) + testsuite, err := evalTestsuite(r, modelSourcePath, ignoreNoqa, useCache, changedFiles) if err != nil { errChan <- err return @@ -269,7 +269,7 @@ func countTotalTestcases(testsuites []Testsuite) int { return count } -func evalTestsuite(rule Rule, modelSourcePath string, ignoreNoqa bool, useCache bool) (*Testsuite, error) { +func evalTestsuite(rule Rule, modelSourcePath string, ignoreNoqa bool, useCache bool, changedFiles []string) (*Testsuite, error) { log.Debugf("evaluating rule %s", rule.Path) @@ -282,6 +282,7 @@ func evalTestsuite(rule Rule, modelSourcePath string, ignoreNoqa bool, useCache if err != nil { return nil, err } + inputFiles = filterInputFiles(inputFiles, normalizeChangedFilesSet(changedFiles)) testcase := &Testcase{} for _, inputFile := range inputFiles { diff --git a/lint/lint_test.go b/lint/lint_test.go index e4540d2..fc26296 100644 --- a/lint/lint_test.go +++ b/lint/lint_test.go @@ -14,7 +14,7 @@ func TestEvalTestsuite_Rego(t *testing.T) { t.Fatalf("Failed to parse rule metadata: %v", err) } - result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false, false) + result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -67,7 +67,7 @@ errors contains "Always fails" Language: LanguageRego, } - result, err := evalTestsuite(rule, tempDir, false, false) + result, err := evalTestsuite(rule, tempDir, false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -85,7 +85,7 @@ func TestEvalTestsuite_Javascript(t *testing.T) { t.Fatalf("Failed to parse rule metadata: %v", err) } - result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false, false) + result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -134,7 +134,7 @@ function rule(input) { Language: LanguageJavascript, } - result, err := evalTestsuite(rule, tempDir, false, false) + result, err := evalTestsuite(rule, tempDir, false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -152,7 +152,7 @@ func TestEvalTestsuite_Typescript(t *testing.T) { t.Fatalf("Failed to parse rule metadata: %v", err) } - result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false, false) + result, err := evalTestsuite(*rule, "./../resources/modelsource-v1", false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -203,7 +203,7 @@ Name: "Test" } t.Run("documentation noqa is ignored", func(t *testing.T) { - result, err := evalTestsuite(rule, tempDir, false, false) + result, err := evalTestsuite(rule, tempDir, false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -217,7 +217,7 @@ Name: "Test" }) t.Run("ignoreNoqa has no effect on documentation skip", func(t *testing.T) { - result, err := evalTestsuite(rule, tempDir, true, false) + result, err := evalTestsuite(rule, tempDir, true, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -275,7 +275,7 @@ function rule(input) { Language: LanguageJavascript, } - result, err := evalTestsuite(rule, tempDir, false, false) + result, err := evalTestsuite(rule, tempDir, false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -483,7 +483,7 @@ function rule(input) { func TestEvalAll(t *testing.T) { t.Run("all rules pass", func(t *testing.T) { - err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", "", "", false, false) + err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", "", "", false, false, nil) if err != nil { t.Errorf("Expected no failures: %v", err) } @@ -493,7 +493,7 @@ func TestEvalAll(t *testing.T) { tempDir := t.TempDir() xunitPath := filepath.Join(tempDir, "report.xml") - err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", xunitPath, "", false, false) + err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", xunitPath, "", false, false, nil) if err != nil { t.Errorf("Expected no failures: %v", err) } @@ -508,7 +508,7 @@ func TestEvalAll(t *testing.T) { tempDir := t.TempDir() jsonPath := filepath.Join(tempDir, "report.json") - err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", "", jsonPath, false, false) + err := EvalAll("./../resources/rules", "./../resources/modelsource-v1", "", jsonPath, false, false, nil) if err != nil { t.Errorf("Expected no failures: %v", err) } @@ -522,7 +522,7 @@ func TestEvalAll(t *testing.T) { func TestEvalAllWithResults(t *testing.T) { t.Run("returns results", func(t *testing.T) { - result, err := EvalAllWithResults("./../resources/rules", "./../resources/modelsource-v1", "", "", false, false) + result, err := EvalAllWithResults("./../resources/rules", "./../resources/modelsource-v1", "", "", false, false, nil) if err != nil { t.Errorf("Expected no failures: %v", err) } @@ -584,7 +584,7 @@ function rule(input) { Language: LanguageJavascript, } - result, err := evalTestsuite(rule, tempDir, false, false) + result, err := evalTestsuite(rule, tempDir, false, false, nil) if err != nil { t.Fatalf("Expected testsuite evaluation to succeed, got: %v", err) } @@ -640,7 +640,7 @@ function rule(input) { Language: LanguageJavascript, } - result, err := evalTestsuite(rule, tempDir, false, false) + result, err := evalTestsuite(rule, tempDir, false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } @@ -683,7 +683,7 @@ function rule(input) { Language: LanguageJavascript, } - result, err := evalTestsuite(rule, tempDir, false, false) + result, err := evalTestsuite(rule, tempDir, false, false, nil) if err != nil { t.Fatalf("Failed to evaluate testsuite: %v", err) } diff --git a/main.go b/main.go index 815969d..ff4d537 100644 --- a/main.go +++ b/main.go @@ -116,6 +116,9 @@ func main() { if !filepath.IsAbs(rulesDirectory) { rulesDirectory = filepath.Join(projectDir, rulesDirectory) } + if !filepath.IsAbs(modelDirectory) { + modelDirectory = filepath.Join(projectDir, modelDirectory) + } if config != nil && len(config.Rules.Rulesets) > 0 { log.Infof("Syncing %d rulesets to %s", len(config.Rules.Rulesets), rulesDirectory) @@ -125,6 +128,36 @@ func main() { } } + diffOnly, err := cmd.Flags().GetBool("diff") + if err != nil { + log.Errorf("failed to read --diff flag: %s", err) + os.Exit(1) + } + + var changedFiles []string + if diffOnly { + changedFiles, err = lint.GitUnstagedChangedFiles(projectDir) + if err != nil { + if err == lint.ErrNotGitRepository { + log.Warnf("--diff ignored: project is not tracked in git; linting all documents") + } else { + log.Errorf("failed to resolve unstaged git changes: %s", err) + os.Exit(1) + } + } else { + changedFiles, err = lint.FilterFilesUnderDirectory(changedFiles, modelDirectory) + if err != nil { + log.Errorf("failed to filter changed files: %s", err) + os.Exit(1) + } + if len(changedFiles) == 0 { + log.Infof("No unstaged changes found in %s; nothing to lint", config.Modelsource) + return + } + log.Infof("Linting %d changed document(s) with unstaged git changes", len(changedFiles)) + } + } + err = lint.EvalAll( rulesDirectory, modelDirectory, @@ -132,6 +165,7 @@ func main() { config.Lint.JSONFile, boolValue(config.Lint.IgnoreNoqa, false), effectiveLintUseCache(config), + changedFiles, ) if err != nil { log.Errorf("lint failed: %s", err) @@ -139,6 +173,7 @@ func main() { } }, } + cmdLint.Flags().Bool("diff", false, "Only lint model documents with unstaged git changes") rootCmd.AddCommand(cmdLint) var cmdConfig = &cobra.Command{ diff --git a/serve/serve.go b/serve/serve.go index 706525b..abeead9 100644 --- a/serve/serve.go +++ b/serve/serve.go @@ -245,6 +245,7 @@ func runServe(cmd *cobra.Command, args []string) { "", boolValue(config.Lint.IgnoreNoqa, false), effectiveLintUseCacheForServe(config), + nil, ) }()