diff --git a/cmd/pint/lint.go b/cmd/pint/lint.go index c02311ae..ce49071d 100644 --- a/cmd/pint/lint.go +++ b/cmd/pint/lint.go @@ -143,8 +143,10 @@ func verifyOwners(entries []discovery.Entry, allowedOwners []*regexp.Regexp) (re } if entry.Owner == "" { reports = append(reports, reporter.Report{ - ReportedPath: entry.ReportedPath, - SourcePath: entry.SourcePath, + Path: discovery.Path{ + Name: entry.Path.Name, + SymlinkTarget: entry.Path.SymlinkTarget, + }, ModifiedLines: entry.ModifiedLines, Rule: entry.Rule, Problem: checks.Problem{ @@ -163,8 +165,10 @@ func verifyOwners(entries []discovery.Entry, allowedOwners []*regexp.Regexp) (re } } reports = append(reports, reporter.Report{ - ReportedPath: entry.ReportedPath, - SourcePath: entry.SourcePath, + Path: discovery.Path{ + Name: entry.Path.Name, + SymlinkTarget: entry.Path.SymlinkTarget, + }, ModifiedLines: entry.ModifiedLines, Rule: entry.Rule, Problem: checks.Problem{ diff --git a/cmd/pint/scan.go b/cmd/pint/scan.go index ba1cf397..523f2d89 100644 --- a/cmd/pint/scan.go +++ b/cmd/pint/scan.go @@ -118,7 +118,7 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr if entry.Rule.RecordingRule != nil { rulesParsedTotal.WithLabelValues(config.RecordingRuleType).Inc() slog.Debug("Found recording rule", - slog.String("path", entry.SourcePath), + slog.String("path", entry.Path.Name), slog.String("record", entry.Rule.RecordingRule.Record.Value), slog.String("lines", entry.Rule.Lines.String()), ) @@ -126,7 +126,7 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr if entry.Rule.AlertingRule != nil { rulesParsedTotal.WithLabelValues(config.AlertingRuleType).Inc() slog.Debug("Found alerting rule", - slog.String("path", entry.SourcePath), + slog.String("path", entry.Path.Name), slog.String("alert", entry.Rule.AlertingRule.Alert.Value), slog.String("lines", entry.Rule.Lines.String()), ) @@ -146,7 +146,7 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr default: if entry.Rule.Error.Err != nil { slog.Debug("Found invalid rule", - slog.String("path", entry.SourcePath), + slog.String("path", entry.Path.Name), slog.String("lines", entry.Rule.Lines.String()), ) rulesParsedTotal.WithLabelValues(config.InvalidRuleType).Inc() @@ -189,8 +189,10 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte switch { case errors.As(job.entry.PathError, &ignoreErr): results <- reporter.Report{ - ReportedPath: job.entry.ReportedPath, - SourcePath: job.entry.SourcePath, + Path: discovery.Path{ + Name: job.entry.Path.Name, + SymlinkTarget: job.entry.Path.SymlinkTarget, + }, ModifiedLines: job.entry.ModifiedLines, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -205,8 +207,10 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte } case errors.As(job.entry.PathError, &commentErr): results <- reporter.Report{ - ReportedPath: job.entry.ReportedPath, - SourcePath: job.entry.SourcePath, + Path: discovery.Path{ + Name: job.entry.Path.Name, + SymlinkTarget: job.entry.Path.SymlinkTarget, + }, ModifiedLines: job.entry.ModifiedLines, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -222,8 +226,10 @@ func scanWorker(ctx context.Context, jobs <-chan scanJob, results chan<- reporte case job.entry.PathError != nil: line, e := tryDecodingYamlError(job.entry.PathError) results <- reporter.Report{ - ReportedPath: job.entry.ReportedPath, - SourcePath: job.entry.SourcePath, + Path: discovery.Path{ + Name: job.entry.Path.Name, + SymlinkTarget: job.entry.Path.SymlinkTarget, + }, ModifiedLines: job.entry.ModifiedLines, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -242,8 +248,10 @@ If this file is a template that will be rendered into valid YAML then you can in } case job.entry.Rule.Error.Err != nil: results <- reporter.Report{ - ReportedPath: job.entry.ReportedPath, - SourcePath: job.entry.SourcePath, + Path: discovery.Path{ + Name: job.entry.Path.Name, + SymlinkTarget: job.entry.Path.SymlinkTarget, + }, ModifiedLines: job.entry.ModifiedLines, Rule: job.entry.Rule, Problem: checks.Problem{ @@ -263,19 +271,21 @@ This usually means that it's missing some required fields.`, if job.entry.State == discovery.Unknown { slog.Warn( "Bug: unknown rule state", - slog.String("path", job.entry.ReportedPath), + slog.String("path", job.entry.Path.String()), slog.Int("line", job.entry.Rule.Lines.First), slog.String("name", job.entry.Rule.Name()), ) } start := time.Now() - problems := job.check.Check(ctx, job.entry.ReportedPath, job.entry.Rule, job.allEntries) + problems := job.check.Check(ctx, job.entry.Path, job.entry.Rule, job.allEntries) checkDuration.WithLabelValues(job.check.Reporter()).Observe(time.Since(start).Seconds()) for _, problem := range problems { results <- reporter.Report{ - ReportedPath: job.entry.ReportedPath, - SourcePath: job.entry.SourcePath, + Path: discovery.Path{ + Name: job.entry.Path.Name, + SymlinkTarget: job.entry.Path.SymlinkTarget, + }, ModifiedLines: job.entry.ModifiedLines, Rule: job.entry.Rule, Problem: problem, diff --git a/cmd/pint/tests/0159_ci_teamcity.txt b/cmd/pint/tests/0159_ci_teamcity.txt index ff6a1515..520b88b0 100644 --- a/cmd/pint/tests/0159_ci_teamcity.txt +++ b/cmd/pint/tests/0159_ci_teamcity.txt @@ -10,17 +10,15 @@ env GIT_COMMITTER_EMAIL=pint@example.com exec git add . exec git commit -am 'import rules and config' -exec git checkout -b v1 +exec git checkout -b pr cp ../src/a.yml a.yml exec git add a.yml exec git commit -am 'v1' -exec git checkout -b v2 cp ../src/b.yml b.yml exec git add b.yml exec git commit -am 'v2' -exec git checkout -b v3 exec git rm a.yml exec git commit -am 'v3' diff --git a/cmd/pint/tests/0171_rule_duplicate_rename.txt b/cmd/pint/tests/0171_rule_duplicate_rename.txt new file mode 100644 index 00000000..ca6af1b3 --- /dev/null +++ b/cmd/pint/tests/0171_rule_duplicate_rename.txt @@ -0,0 +1,64 @@ +http response prometheus /api/v1/status/flags 200 {"status":"success","data":{"storage.tsdb.retention.time": "1d"}} +http response prometheus /api/v1/metadata 200 {"status":"success","data":{}} +http response prometheus /api/v1/status/config 200 {"status":"success","data":{"yaml":"global:\n scrape_interval: 1m\n"}} +http response prometheus /api/v1/query_range 200 {"status":"success","data":{"resultType":"matrix","result":[]}} +http response prometheus /api/v1/query 200 {"status":"success","data":{"resultType":"vector","result":[]}} +http start prometheus 127.0.0.1:7171 + +mkdir testrepo +cd testrepo +exec git init --initial-branch=main . + +cp ../src/.pint.hcl . +cp ../src/v1.yaml rules.yaml +env GIT_AUTHOR_NAME=pint +env GIT_AUTHOR_EMAIL=pint@example.com +env GIT_COMMITTER_NAME=pint +env GIT_COMMITTER_EMAIL=pint@example.com +exec git add . +exec git commit -am 'import rules and config' + +exec git checkout -b pr + +cp ../src/v2.yaml rules.yaml +exec git commit -am 'v2' + +exec git mv rules.yaml renamed.yaml +exec git commit -am 'v3' + +pint.error --no-color ci +! stdout . +cmp stderr ../stderr.txt + +-- stderr.txt -- +level=INFO msg="Loading configuration file" path=.pint.hcl +level=INFO msg="Finding all rules to check on current git branch" base=main +level=INFO msg="Configured new Prometheus server" name=prom uris=1 uptime=up tags=[] include=["^.*.yaml$"] exclude=[] +level=WARN msg="No results for Prometheus uptime metric, you might have set uptime config option to a missing metric, please check your config" name=prom metric=up +level=WARN msg="Using dummy Prometheus uptime metric results with no gaps" name=prom metric=up +level=INFO msg="Problems found" Bug=1 +renamed.yaml:2 Bug: `prom` Prometheus server at http://127.0.0.1:7171 didn't have any series for `foo` metric in the last 1w. (promql/series) + 2 | expr: sum(foo) + +level=ERROR msg="Fatal error" err="problems found" +-- src/v1.yaml -- +- record: rule1 + # pint disable promql/series + expr: sum(foo) +-- src/v2.yaml -- +- record: rule1 + expr: sum(foo) +-- src/.pint.hcl -- +ci { + baseBranch = "main" +} +parser { + relaxed = [".*"] +} +prometheus "prom" { + uri = "http://127.0.0.1:7171" + failover = [] + timeout = "5s" + required = true + include = [".*.yaml"] +} diff --git a/cmd/pint/tests/0172_rule_dependency_symlink_delete.txt b/cmd/pint/tests/0172_rule_dependency_symlink_delete.txt new file mode 100644 index 00000000..86c7c9e6 --- /dev/null +++ b/cmd/pint/tests/0172_rule_dependency_symlink_delete.txt @@ -0,0 +1,56 @@ +mkdir testrepo +cd testrepo +exec git init --initial-branch=main . + +mkdir rules1 rules2 +cp ../src/alert.yml rules1/alert.yml +cp ../src/record.yml rules1/record.yml +exec ln -s ../rules1/alert.yml rules2/alert.yml +exec ln -s ../rules1/record.yml rules2/record.yml +cp ../src/.pint.hcl . +env GIT_AUTHOR_NAME=pint +env GIT_AUTHOR_EMAIL=pint@example.com +env GIT_COMMITTER_NAME=pint +env GIT_COMMITTER_EMAIL=pint@example.com +exec git add . +exec git commit -am 'import rules and config' + +exec git checkout -b v2 +exec git rm -fr rules2 +exec git commit -am 'v2' + +pint.ok -l error --offline --no-color ci +! stdout . +cmp stderr ../stderr.txt + +-- stderr.txt -- +-- src/alert.yml -- +groups: +- name: g1 + rules: + - alert: Alert + expr: 'up:sum == 0' + annotations: + summary: 'Service is down' +-- src/record.yml -- +groups: +- name: g1 + rules: + - record: up:sum + expr: sum(up) +-- src/.pint.hcl -- +ci { + baseBranch = "main" +} +prometheus "prom1" { + uri = "http://127.0.0.1:7172/1" + timeout = "5s" + required = true + include = ["rules1/.*"] +} +prometheus "prom2" { + uri = "http://127.0.0.1:7172/2" + timeout = "5s" + required = true + include = ["rules2/.*"] +} diff --git a/cmd/pint/watch.go b/cmd/pint/watch.go index 6362058e..dfa7d643 100644 --- a/cmd/pint/watch.go +++ b/cmd/pint/watch.go @@ -326,7 +326,7 @@ func (c *problemCollector) scan(ctx context.Context, workers int, isOffline bool fileOwners := map[string]string{} for _, entry := range entries { if entry.Owner != "" { - fileOwners[entry.ReportedPath] = entry.Owner + fileOwners[entry.Path.SymlinkTarget] = entry.Owner } } c.fileOwners = fileOwners @@ -377,7 +377,7 @@ func (c *problemCollector) Collect(ch chan<- prometheus.Metric) { c.problem, prometheus.GaugeValue, 1, - report.SourcePath, + report.Path.Name, kind, name, strings.ToLower(report.Problem.Severity.String()), diff --git a/docs/changelog.md b/docs/changelog.md index c0f7bd4f..fcf7de16 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,13 @@ # Changelog +## v0.58.0 + +### Fixed + +- Fixed false positives from [rule/duplicate](checks/rule/duplicate.md) check + when running `pint ci` on files that are both edited and renamed in the same PR. +- [rule/dependency](checks/rule/dependency.md) will no longer report removed symlinks. + ## v0.57.3 ### Fixed diff --git a/internal/checks/alerts_annotation.go b/internal/checks/alerts_annotation.go index 21b79b02..0a74af03 100644 --- a/internal/checks/alerts_annotation.go +++ b/internal/checks/alerts_annotation.go @@ -61,7 +61,7 @@ func (c AnnotationCheck) Reporter() string { return AnnotationCheckName } -func (c AnnotationCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c AnnotationCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil { return nil } diff --git a/internal/checks/alerts_comparison.go b/internal/checks/alerts_comparison.go index c3ca96b6..564364dd 100644 --- a/internal/checks/alerts_comparison.go +++ b/internal/checks/alerts_comparison.go @@ -45,7 +45,7 @@ func (c ComparisonCheck) Reporter() string { return ComparisonCheckName } -func (c ComparisonCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c ComparisonCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil { return problems } diff --git a/internal/checks/alerts_count.go b/internal/checks/alerts_count.go index a109deda..44d58619 100644 --- a/internal/checks/alerts_count.go +++ b/internal/checks/alerts_count.go @@ -61,7 +61,7 @@ func (c AlertsCheck) Reporter() string { return AlertsCheckName } -func (c AlertsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c AlertsCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil { return problems } diff --git a/internal/checks/alerts_external_labels.go b/internal/checks/alerts_external_labels.go index 53f17d0d..c01a3240 100644 --- a/internal/checks/alerts_external_labels.go +++ b/internal/checks/alerts_external_labels.go @@ -43,7 +43,7 @@ func (c AlertsExternalLabelsCheck) Reporter() string { return AlertsExternalLabelsCheckName } -func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c AlertsExternalLabelsCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil { return problems } diff --git a/internal/checks/alerts_for.go b/internal/checks/alerts_for.go index 66041c07..70eddfbb 100644 --- a/internal/checks/alerts_for.go +++ b/internal/checks/alerts_for.go @@ -41,7 +41,7 @@ func (c AlertsForChecksFor) Reporter() string { return AlertForCheckName } -func (c AlertsForChecksFor) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c AlertsForChecksFor) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil { return problems } diff --git a/internal/checks/alerts_template.go b/internal/checks/alerts_template.go index 2c90ce79..057cbf6b 100644 --- a/internal/checks/alerts_template.go +++ b/internal/checks/alerts_template.go @@ -110,7 +110,7 @@ func (c TemplateCheck) Reporter() string { return TemplateCheckName } -func (c TemplateCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c TemplateCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil { return nil } diff --git a/internal/checks/base.go b/internal/checks/base.go index 91f98a54..b86c96c1 100644 --- a/internal/checks/base.go +++ b/internal/checks/base.go @@ -123,7 +123,7 @@ type RuleChecker interface { String() string Reporter() string Meta() CheckMeta - Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) []Problem + Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) []Problem } type exprProblem struct { diff --git a/internal/checks/base_test.go b/internal/checks/base_test.go index a19b3a1b..2481f851 100644 --- a/internal/checks/base_test.go +++ b/internal/checks/base_test.go @@ -143,7 +143,7 @@ func runTests(t *testing.T, testCases []checkTest) { if tc.ctx != nil { ctx = tc.ctx() } - problems := tc.checker(prom).Check(ctx, entry.SourcePath, entry.Rule, tc.entries) + problems := tc.checker(prom).Check(ctx, entry.Path, entry.Rule, tc.entries) require.Equal(t, tc.problems(uri), problems) } @@ -166,7 +166,7 @@ func runTests(t *testing.T, testCases []checkTest) { require.NoError(t, err, "cannot parse rule content") t.Run(tc.description+" (bogus rules)", func(_ *testing.T) { for _, entry := range entries { - _ = tc.checker(newSimpleProm("prom")).Check(context.Background(), entry.SourcePath, entry.Rule, tc.entries) + _ = tc.checker(newSimpleProm("prom")).Check(context.Background(), entry.Path, entry.Rule, tc.entries) } }) } @@ -181,8 +181,10 @@ func parseContent(content string) (entries []discovery.Entry, err error) { for _, rule := range rules { entries = append(entries, discovery.Entry{ - SourcePath: "fake.yml", - ReportedPath: "fake.yml", + Path: discovery.Path{ + Name: "fake.yml", + SymlinkTarget: "fake.yml", + }, ModifiedLines: rule.Lines.Expand(), Rule: rule, }) diff --git a/internal/checks/labels_conflict.go b/internal/checks/labels_conflict.go index e4cb7ff2..360e6acd 100644 --- a/internal/checks/labels_conflict.go +++ b/internal/checks/labels_conflict.go @@ -41,7 +41,7 @@ func (c LabelsConflictCheck) Reporter() string { return LabelsConflictCheckName } -func (c LabelsConflictCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c LabelsConflictCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { var labels *parser.YamlMap if rule.AlertingRule != nil && rule.AlertingRule.Expr.SyntaxError == nil && rule.AlertingRule.Labels != nil { labels = rule.AlertingRule.Labels diff --git a/internal/checks/promql_aggregation.go b/internal/checks/promql_aggregation.go index aa73c32b..4aff2f57 100644 --- a/internal/checks/promql_aggregation.go +++ b/internal/checks/promql_aggregation.go @@ -53,7 +53,7 @@ func (c AggregationCheck) Reporter() string { return AggregationCheckName } -func (c AggregationCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c AggregationCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { return nil diff --git a/internal/checks/promql_counter.go b/internal/checks/promql_counter.go index 409e8ac1..59628693 100644 --- a/internal/checks/promql_counter.go +++ b/internal/checks/promql_counter.go @@ -48,7 +48,7 @@ func (c CounterCheck) Reporter() string { return CounterCheckName } -func (c CounterCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c CounterCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { diff --git a/internal/checks/promql_fragile.go b/internal/checks/promql_fragile.go index aa5a3aa0..07a53282 100644 --- a/internal/checks/promql_fragile.go +++ b/internal/checks/promql_fragile.go @@ -40,7 +40,7 @@ func (c FragileCheck) Reporter() string { return FragileCheckName } -func (c FragileCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c FragileCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { return nil diff --git a/internal/checks/promql_range_query.go b/internal/checks/promql_range_query.go index c3749611..ddf2770e 100644 --- a/internal/checks/promql_range_query.go +++ b/internal/checks/promql_range_query.go @@ -45,7 +45,7 @@ func (c RangeQueryCheck) Reporter() string { return RangeQueryCheckName } -func (c RangeQueryCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c RangeQueryCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { diff --git a/internal/checks/promql_rate.go b/internal/checks/promql_rate.go index 4b460fc9..188c8131 100644 --- a/internal/checks/promql_rate.go +++ b/internal/checks/promql_rate.go @@ -60,7 +60,7 @@ func (c RateCheck) Reporter() string { return RateCheckName } -func (c RateCheck) Check(ctx context.Context, _ string, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { +func (c RateCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { diff --git a/internal/checks/promql_regexp.go b/internal/checks/promql_regexp.go index 2d8344dd..55fdd4f3 100644 --- a/internal/checks/promql_regexp.go +++ b/internal/checks/promql_regexp.go @@ -44,7 +44,7 @@ func (c RegexpCheck) Reporter() string { return RegexpCheckName } -func (c RegexpCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c RegexpCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { return nil diff --git a/internal/checks/promql_series.go b/internal/checks/promql_series.go index 09554246..ca8f43f2 100644 --- a/internal/checks/promql_series.go +++ b/internal/checks/promql_series.go @@ -100,7 +100,7 @@ func (c SeriesCheck) Reporter() string { return SeriesCheckName } -func (c SeriesCheck) Check(ctx context.Context, _ string, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { +func (c SeriesCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { var settings *PromqlSeriesSettings if s := ctx.Value(SettingsKey(c.Reporter())); s != nil { settings = s.(*PromqlSeriesSettings) @@ -163,7 +163,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ string, rule parser.Rule, entr slog.Debug( "Metric is provided by alerting rule", slog.String("selector", (&selector).String()), - slog.String("path", arEntry.SourcePath), + slog.String("path", arEntry.Path.Name), ) } else { problems = append(problems, Problem{ @@ -262,7 +262,7 @@ func (c SeriesCheck) Check(ctx context.Context, _ string, rule parser.Rule, entr } if rrEntry != nil { // Validate recording rule instead - slog.Debug("Metric is provided by recording rule", slog.String("selector", (&bareSelector).String()), slog.String("path", rrEntry.SourcePath)) + slog.Debug("Metric is provided by recording rule", slog.String("selector", (&bareSelector).String()), slog.String("path", rrEntry.Path.Name)) problems = append(problems, Problem{ Lines: expr.Value.Lines, Reporter: c.Reporter(), diff --git a/internal/checks/promql_syntax.go b/internal/checks/promql_syntax.go index 8e40ba58..9f6a2eee 100644 --- a/internal/checks/promql_syntax.go +++ b/internal/checks/promql_syntax.go @@ -39,7 +39,7 @@ func (c SyntaxCheck) Reporter() string { return SyntaxCheckName } -func (c SyntaxCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c SyntaxCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { problems = append(problems, Problem{ diff --git a/internal/checks/promql_vector_matching.go b/internal/checks/promql_vector_matching.go index f71464fb..8fe591d8 100644 --- a/internal/checks/promql_vector_matching.go +++ b/internal/checks/promql_vector_matching.go @@ -52,7 +52,7 @@ func (c VectorMatchingCheck) Reporter() string { return VectorMatchingCheckName } -func (c VectorMatchingCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c VectorMatchingCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { return nil diff --git a/internal/checks/query_cost.go b/internal/checks/query_cost.go index ece99c42..049e93cc 100644 --- a/internal/checks/query_cost.go +++ b/internal/checks/query_cost.go @@ -61,7 +61,7 @@ func (c CostCheck) Reporter() string { return CostCheckName } -func (c CostCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c CostCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { expr := rule.Expr() if expr.SyntaxError != nil { diff --git a/internal/checks/rule_dependency.go b/internal/checks/rule_dependency.go index ab4af887..4813523b 100644 --- a/internal/checks/rule_dependency.go +++ b/internal/checks/rule_dependency.go @@ -41,7 +41,12 @@ func (c RuleDependencyCheck) Reporter() string { return RuleDependencyCheckName } -func (c RuleDependencyCheck) Check(_ context.Context, _ string, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { +func (c RuleDependencyCheck) Check(_ context.Context, path discovery.Path, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { + if path.Name != path.SymlinkTarget { + // Don't reported symlinks that are being removed. + return problems + } + var broken []*brokenDependency var dep *brokenDependency for _, entry := range entries { @@ -130,7 +135,7 @@ func (c RuleDependencyCheck) usesVector(entry discovery.Entry, name string) *bro return &brokenDependency{ kind: "recording", metric: name, - path: entry.ReportedPath, + path: entry.Path.SymlinkTarget, line: expr.Value.Lines.First, name: entry.Rule.Name(), } @@ -155,7 +160,7 @@ func (c RuleDependencyCheck) usesAlert(entry discovery.Entry, name string) *brok return &brokenDependency{ kind: "alerting", metric: vs.String(), - path: entry.ReportedPath, + path: entry.Path.SymlinkTarget, line: expr.Value.Lines.First, name: entry.Rule.Name(), } diff --git a/internal/checks/rule_dependency_test.go b/internal/checks/rule_dependency_test.go index 6dba089a..f88e6295 100644 --- a/internal/checks/rule_dependency_test.go +++ b/internal/checks/rule_dependency_test.go @@ -29,8 +29,8 @@ func TestRuleDependencyCheck(t *testing.T) { entries := mustParseContent(input) for i := range entries { entries[i].State = state - entries[i].SourcePath = sp - entries[i].ReportedPath = rp + entries[i].Path.Name = sp + entries[i].Path.SymlinkTarget = rp } return entries @@ -158,9 +158,11 @@ func TestRuleDependencyCheck(t *testing.T) { problems: noProblems, entries: []discovery.Entry{ { - ReportedPath: "broken.yaml", - SourcePath: "broken.yaml", - PathError: errors.New("bad file"), + Path: discovery.Path{ + Name: "broken.yaml", + SymlinkTarget: "broken.yaml", + }, + PathError: errors.New("bad file"), }, parseWithState("- alert: foo\n expr: up == 0\n", discovery.Noop, "foo.yaml", "foo.yaml")[0], }, diff --git a/internal/checks/rule_duplicate.go b/internal/checks/rule_duplicate.go index dbe16fe7..c2e7a381 100644 --- a/internal/checks/rule_duplicate.go +++ b/internal/checks/rule_duplicate.go @@ -43,7 +43,7 @@ func (c RuleDuplicateCheck) Reporter() string { return RuleDuplicateCheckName } -func (c RuleDuplicateCheck) Check(ctx context.Context, path string, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { +func (c RuleDuplicateCheck) Check(ctx context.Context, path discovery.Path, rule parser.Rule, entries []discovery.Entry) (problems []Problem) { if rule.RecordingRule == nil || rule.RecordingRule.Expr.SyntaxError != nil { return nil } @@ -55,13 +55,13 @@ func (c RuleDuplicateCheck) Check(ctx context.Context, path string, rule parser. if entry.Rule.RecordingRule == nil { continue } - if entry.SourcePath == path && entry.Rule.Lines.First == rule.Lines.First { + if entry.Path.Name == path.Name && entry.Rule.Lines.First == rule.Lines.First { continue } - if !c.prom.IsEnabledForPath(path) { + if !c.prom.IsEnabledForPath(path.Name) { continue } - if !c.prom.IsEnabledForPath(entry.SourcePath) { + if !c.prom.IsEnabledForPath(entry.Path.Name) { continue } if entry.Rule.RecordingRule.Record.Value != rule.RecordingRule.Record.Value { @@ -84,7 +84,7 @@ func (c RuleDuplicateCheck) compareRules(_ context.Context, rule *parser.Recordi problems = append(problems, Problem{ Lines: lines, Reporter: c.Reporter(), - Text: fmt.Sprintf("Duplicated rule, identical rule found at %s:%d.", entry.ReportedPath, entry.Rule.RecordingRule.Record.Lines.First), + Text: fmt.Sprintf("Duplicated rule, identical rule found at %s:%d.", entry.Path.SymlinkTarget, entry.Rule.RecordingRule.Record.Lines.First), Severity: Bug, }) } diff --git a/internal/checks/rule_duplicate_test.go b/internal/checks/rule_duplicate_test.go index 6dc02bc7..0a99d658 100644 --- a/internal/checks/rule_duplicate_test.go +++ b/internal/checks/rule_duplicate_test.go @@ -18,7 +18,7 @@ func textDuplicateRule(path string, line int) string { func TestRuleDuplicateCheck(t *testing.T) { xxxEntries := mustParseContent("- record: foo\n expr: up == 0\n") for i := range xxxEntries { - xxxEntries[i].ReportedPath = "xxx.yml" + xxxEntries[i].Path.SymlinkTarget = "xxx.yml" } testCases := []checkTest{ diff --git a/internal/checks/rule_for.go b/internal/checks/rule_for.go index e0c5fd77..5f54b510 100644 --- a/internal/checks/rule_for.go +++ b/internal/checks/rule_for.go @@ -61,7 +61,7 @@ func (c RuleForCheck) Reporter() string { return RuleForCheckName } -func (c RuleForCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c RuleForCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil { return nil } diff --git a/internal/checks/rule_label.go b/internal/checks/rule_label.go index d6e1fb7b..0bb6afc8 100644 --- a/internal/checks/rule_label.go +++ b/internal/checks/rule_label.go @@ -60,7 +60,7 @@ func (c LabelCheck) Reporter() string { return LabelCheckName } -func (c LabelCheck) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c LabelCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.RecordingRule != nil { problems = append(problems, c.checkRecordingRule(rule)...) } diff --git a/internal/checks/rule_link.go b/internal/checks/rule_link.go index 00bb97f5..c4add066 100644 --- a/internal/checks/rule_link.go +++ b/internal/checks/rule_link.go @@ -62,7 +62,7 @@ func (c RuleLinkCheck) Reporter() string { return RuleLinkCheckName } -func (c RuleLinkCheck) Check(ctx context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c RuleLinkCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if rule.AlertingRule == nil || rule.AlertingRule.Annotations == nil { return nil } diff --git a/internal/checks/rule_reject.go b/internal/checks/rule_reject.go index 3b3e46bb..77c23e98 100644 --- a/internal/checks/rule_reject.go +++ b/internal/checks/rule_reject.go @@ -52,7 +52,7 @@ func (c Reject) Reporter() string { return RejectCheckName } -func (c Reject) Check(_ context.Context, _ string, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { +func (c Reject) Check(_ context.Context, _ discovery.Path, rule parser.Rule, _ []discovery.Entry) (problems []Problem) { if c.checkLabels && rule.AlertingRule != nil && rule.AlertingRule.Labels != nil { for _, label := range rule.AlertingRule.Labels.Items { problems = append(problems, c.reject(rule, label, "Label", label.Value.Lines)...) diff --git a/internal/config/config.go b/internal/config/config.go index 792dd317..132d2e88 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -114,7 +114,7 @@ func (cfg *Config) GetChecksForRule(ctx context.Context, gen *PrometheusGenerato }, } - proms := gen.ServersForPath(entry.SourcePath) + proms := gen.ServersForPath(entry.Path.Name) for _, p := range proms { allChecks = append(allChecks, checkMeta{ @@ -160,7 +160,7 @@ func (cfg *Config) GetChecksForRule(ctx context.Context, gen *PrometheusGenerato } for _, rule := range cfg.Rules { - allChecks = append(allChecks, rule.resolveChecks(ctx, entry.SourcePath, entry.Rule, proms)...) + allChecks = append(allChecks, rule.resolveChecks(ctx, entry.Path.Name, entry.Rule, proms)...) } for _, cm := range allChecks { @@ -197,7 +197,7 @@ func (cfg *Config) GetChecksForRule(ctx context.Context, gen *PrometheusGenerato } slog.Debug("Configured checks for rule", slog.Any("enabled", el), - slog.String("path", entry.SourcePath), + slog.String("path", entry.Path.Name), slog.String("rule", entry.Rule.Name()), ) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index a1f898e3..36e5e687 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -156,9 +156,12 @@ func TestGetChecksForRule(t *testing.T) { title: "defaults", config: "", entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -178,9 +181,12 @@ prometheus "prom" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -208,9 +214,12 @@ prometheus "prom" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -244,8 +253,11 @@ checks { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # pint disable promql/counter # pint disable promql/rate @@ -276,9 +288,12 @@ prometheus "prom" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -300,9 +315,12 @@ prometheus "prom" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -323,9 +341,12 @@ prometheus "prom" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -346,9 +367,12 @@ prometheus "prom" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -381,9 +405,12 @@ prometheus "ignore" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -405,9 +432,12 @@ prometheus "ignore" { title: "single empty rule", config: "rule{}\n", entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -432,9 +462,12 @@ rule { } }`, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -462,8 +495,11 @@ rule { } }`, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` - record: foo # pint disable promql/aggregate(instance:false) @@ -492,9 +528,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -523,8 +562,11 @@ prometheus "prom2" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # pint disable promql/series(prom1) # pint disable query/cost(prom2) @@ -601,9 +643,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -650,8 +695,11 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # pint disable promql/counter # pint disable promql/series @@ -702,9 +750,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -736,9 +787,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -767,9 +821,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -798,9 +855,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: dev\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: dev\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -829,9 +889,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: prod\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n labels:\n cluster: prod\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -860,9 +923,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -891,9 +957,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n annotations:\n cluster: dev\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n annotations:\n cluster: dev\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -924,9 +993,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n annotations:\n cluster: prod\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n annotations:\n cluster: prod\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -964,8 +1036,11 @@ prometheus "prom1" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` - record: foo expr: sum(foo) @@ -1003,8 +1078,11 @@ prometheus "prom1" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` - record: foo expr: sum(foo) @@ -1051,8 +1129,11 @@ prometheus "prom1" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` - record: foo expr: sum(foo) @@ -1089,8 +1170,11 @@ checks { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` - alert: foo expr: sum(foo) @@ -1127,8 +1211,11 @@ checks { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` - alert: foo expr: sum(foo) @@ -1151,9 +1238,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1179,9 +1269,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 14m\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 14m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1205,9 +1298,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n keep_firing_for: 16m\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n keep_firing_for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1232,9 +1328,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1258,9 +1357,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n keep_firing_for: 14m\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n keep_firing_for: 14m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1284,9 +1386,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1310,9 +1415,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 16m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1338,9 +1446,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 14m\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- alert: foo\n expr: sum(foo)\n for: 14m\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1364,9 +1475,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1394,9 +1508,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1424,8 +1541,11 @@ checks { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # Some extra comment # pint disable promql/series @@ -1460,8 +1580,11 @@ checks { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # pint snooze 2099-11-AB labels/conflict # pint snooze 2099-11-28 labels/conflict won't work @@ -1505,8 +1628,11 @@ checks { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # pint snooze 2000-11-28 promql/series(prom1) # pint snooze 2000-11-28T10:24:18Z promql/range_query @@ -1556,8 +1682,11 @@ prometheus "prom3" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # pint disable alerts/count(+disable) # pint disable alerts/external_labels(+disable) @@ -1614,8 +1743,11 @@ prometheus "prom3" { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, Rule: newRule(t, ` # pint snooze 2099-11-28 alerts/count(+disable) # pint snooze 2099-11-28 alerts/external_labels(+disable) @@ -1672,9 +1804,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, @@ -1713,9 +1848,12 @@ rule { } `, entry: discovery.Entry{ - State: discovery.Modified, - SourcePath: "rules.yml", - Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + Rule: newRule(t, "- record: foo\n expr: sum(foo)\n"), }, checks: []string{ checks.SyntaxCheckName, diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go index a14d945e..be5fa005 100644 --- a/internal/discovery/discovery.go +++ b/internal/discovery/discovery.go @@ -3,6 +3,7 @@ package discovery import ( "encoding/json" "errors" + "fmt" "io" "log/slog" "strings" @@ -89,10 +90,21 @@ const ( Excluded ) +type Path struct { + Name string // file path, it can be symlink + SymlinkTarget string // symlink target, or the same as name if not a symlink +} + +func (p Path) String() string { + if p.Name == p.SymlinkTarget { + return p.Name + } + return fmt.Sprintf("%s ~> %s", p.Name, p.SymlinkTarget) +} + type Entry struct { PathError error - ReportedPath string // symlink target - SourcePath string // file path (can be symlink) + Path Path Owner string ModifiedLines []int DisabledChecks []string @@ -142,8 +154,10 @@ func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (ent ) case comments.InvalidComment: entries = append(entries, Entry{ - ReportedPath: reportedPath, - SourcePath: sourcePath, + Path: Path{ + Name: sourcePath, + SymlinkTarget: reportedPath, + }, PathError: comment.Value.(comments.Invalid).Err, Owner: fileOwner, ModifiedLines: contentLines.Expand(), @@ -153,8 +167,10 @@ func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (ent if content.Ignored { entries = append(entries, Entry{ - ReportedPath: reportedPath, - SourcePath: sourcePath, + Path: Path{ + Name: sourcePath, + SymlinkTarget: reportedPath, + }, PathError: FileIgnoreError{ Line: content.IgnoreLine, // nolint:revive @@ -178,8 +194,10 @@ func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (ent } seen[err.Error()] = struct{}{} entries = append(entries, Entry{ - ReportedPath: reportedPath, - SourcePath: sourcePath, + Path: Path{ + Name: sourcePath, + SymlinkTarget: reportedPath, + }, PathError: err, Owner: fileOwner, ModifiedLines: contentLines.Expand(), @@ -200,8 +218,10 @@ func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (ent slog.String("lines", contentLines.String()), ) entries = append(entries, Entry{ - ReportedPath: reportedPath, - SourcePath: sourcePath, + Path: Path{ + Name: sourcePath, + SymlinkTarget: reportedPath, + }, PathError: err, Owner: fileOwner, ModifiedLines: contentLines.Expand(), @@ -215,8 +235,10 @@ func readRules(reportedPath, sourcePath string, r io.Reader, isStrict bool) (ent ruleOwner = owner.Name } entries = append(entries, Entry{ - ReportedPath: reportedPath, - SourcePath: sourcePath, + Path: Path{ + Name: sourcePath, + SymlinkTarget: reportedPath, + }, Rule: rule, ModifiedLines: rule.Lines.Expand(), Owner: ruleOwner, diff --git a/internal/discovery/discovery_test.go b/internal/discovery/discovery_test.go index 60862100..c245a6a3 100644 --- a/internal/discovery/discovery_test.go +++ b/internal/discovery/discovery_test.go @@ -130,9 +130,11 @@ func TestReadRules(t *testing.T) { isStrict: false, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{4, 5}, Rule: mustParse(3, "- record: foo\n expr: bar\n"), DisabledChecks: []string{"promql/series"}, @@ -157,9 +159,11 @@ groups: isStrict: true, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{7, 8}, Rule: mustParse(6, "- record: foo\n expr: bar\n"), DisabledChecks: []string{"promql/series"}, @@ -181,9 +185,11 @@ groups: isStrict: false, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{4, 5}, Rule: mustParse(3, "- record: foo\n expr: bar\n"), }, @@ -207,9 +213,11 @@ groups: isStrict: true, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{7, 8}, Rule: mustParse(6, "- record: foo\n expr: bar\n"), }, @@ -230,9 +238,11 @@ groups: isStrict: false, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{4, 5}, Rule: mustParse(3, "- record: foo\n expr: bar\n"), DisabledChecks: []string{"promql/series"}, @@ -257,9 +267,11 @@ groups: isStrict: true, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{7, 8}, Rule: mustParse(6, "- record: foo\n expr: bar\n"), DisabledChecks: []string{"promql/series"}, @@ -281,9 +293,11 @@ groups: isStrict: false, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{1, 2, 3, 4, 5}, PathError: FileIgnoreError{Line: 2, Err: errors.New("file was ignored")}, }, @@ -307,9 +321,11 @@ groups: isStrict: true, entries: []Entry{ { - State: Unknown, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: Unknown, + Path: Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{1, 2, 3, 4, 5, 6, 7, 8}, PathError: FileIgnoreError{Line: 2, Err: errors.New("file was ignored")}, }, diff --git a/internal/discovery/git_branch.go b/internal/discovery/git_branch.go index c676d7d4..90b9f44e 100644 --- a/internal/discovery/git_branch.go +++ b/internal/discovery/git_branch.go @@ -89,7 +89,7 @@ func (f GitBranchFinder) Find(allEntries []Entry) (entries []Entry, err error) { "Rule added on HEAD branch", slog.String("name", me.after.Rule.Name()), slog.String("state", me.after.State.String()), - slog.String("path", me.after.SourcePath), + slog.String("path", me.after.Path.Name), slog.String("ruleLines", me.after.Rule.Lines.String()), slog.String("modifiedLines", output.FormatLineRangeString(me.after.ModifiedLines)), ) @@ -117,7 +117,7 @@ func (f GitBranchFinder) Find(allEntries []Entry) (entries []Entry, err error) { "Rule modified on HEAD branch", slog.String("name", me.after.Rule.Name()), slog.String("state", me.after.State.String()), - slog.String("path", me.after.SourcePath), + slog.String("path", me.after.Path.Name), slog.String("ruleLines", me.after.Rule.Lines.String()), slog.String("modifiedLines", output.FormatLineRangeString(me.after.ModifiedLines)), ) @@ -135,16 +135,16 @@ func (f GitBranchFinder) Find(allEntries []Entry) (entries []Entry, err error) { "Rule removed on HEAD branch", slog.String("name", me.before.Rule.Name()), slog.String("state", me.before.State.String()), - slog.String("path", me.before.SourcePath), + slog.String("path", me.before.Path.Name), slog.String("ruleLines", me.before.Rule.Lines.String()), slog.String("modifiedLines", output.FormatLineRangeString(me.before.ModifiedLines)), ) entries = append(entries, me.before) default: - slog.Debug( - "Unknown rule", + slog.Warn( + "Unknown rule state", slog.String("state", me.before.State.String()), - slog.String("path", me.before.SourcePath), + slog.String("path", me.before.Path.Name), slog.String("modifiedLines", output.FormatLineRangeString(me.before.ModifiedLines)), ) entries = append(entries, me.after) @@ -158,7 +158,7 @@ func (f GitBranchFinder) Find(allEntries []Entry) (entries []Entry, err error) { } for _, entry := range symlinks { - if f.filter.IsPathAllowed(entry.SourcePath) { + if f.filter.IsPathAllowed(entry.Path.Name) { entries = append(entries, entry) } } @@ -170,7 +170,7 @@ func (f GitBranchFinder) Find(allEntries []Entry) (entries []Entry, err error) { goto NEXT } for i, globEntry := range allEntries { - if entry.SourcePath == globEntry.SourcePath && entry.Rule.IsSame(globEntry.Rule) { + if entry.Path.Name == globEntry.Path.Name && entry.Rule.IsSame(globEntry.Rule) { allEntries[i].State = entry.State allEntries[i].ModifiedLines = entry.ModifiedLines found = true @@ -238,6 +238,13 @@ type matchedEntry struct { func matchEntries(before, after []Entry) (ml []matchedEntry) { for _, a := range after { + slog.Debug( + "Matching HEAD rule", + slog.String("path", a.Path.Name), + slog.String("source", a.Path.SymlinkTarget), + slog.String("name", a.Rule.Name()), + ) + m := matchedEntry{after: a, hasAfter: true} beforeSwap := make([]Entry, 0, len(before)) var matches []Entry @@ -248,8 +255,13 @@ func matchEntries(before, after []Entry) (ml []matchedEntry) { m.before = b m.hasBefore = true m.isIdentical = true - m.wasMoved = a.SourcePath != b.SourcePath + m.wasMoved = a.Path.Name != b.Path.Name matched = true + slog.Debug( + "Found identical rule on before & after", + slog.Bool("identical", m.isIdentical), + slog.Bool("moved", m.wasMoved), + ) } else { beforeSwap = append(beforeSwap, b) } @@ -263,7 +275,13 @@ func matchEntries(before, after []Entry) (ml []matchedEntry) { case 1: m.before = matches[0] m.hasBefore = true + m.wasMoved = a.Path.Name != matches[0].Path.Name + slog.Debug("Found rule with same name on before & after") default: + slog.Debug( + "Found multiple rules with same name on before & after", + slog.Int("matches", len(matches)), + ) before = append(before, matches...) } } diff --git a/internal/discovery/git_branch_test.go b/internal/discovery/git_branch_test.go index 20de6c41..c1eb4777 100644 --- a/internal/discovery/git_branch_test.go +++ b/internal/discovery/git_branch_test.go @@ -220,9 +220,11 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, nil), "main", 4), entries: []discovery.Entry{ { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(4, "- record: up:count\n expr: count(up == 1)\n"), }, @@ -253,9 +255,11 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, nil), "main", 4), entries: []discovery.Entry{ { - State: discovery.Modified, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{6}, Rule: mustParse(4, "- record: up:count\n expr: count(up == 1)\n"), }, @@ -280,9 +284,11 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Modified, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{3}, Rule: mustParse(1, "- record: up:count\n expr: count(up == 1)\n"), }, @@ -307,9 +313,11 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(nil, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Modified, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{3}, Rule: mustParse(1, "- record: up:count\n expr: count(up == 1)\n"), }, @@ -415,9 +423,11 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "symlink.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "symlink.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{2, 3}, Rule: mustParse(1, "- record: up:count\n expr: count(up)\n"), }, @@ -458,37 +468,47 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, nil), "main", 4), entries: []discovery.Entry{ { - State: discovery.Modified, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{6}, Rule: mustParse(4, "- record: up:count:1\n expr: count(up == 1)\n"), }, { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{7}, Rule: mustParse(6, "- record: up:count:2a\n expr: count(up)\n"), }, { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(8, "- record: up:count:3\n expr: count(up)\n"), }, { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{11, 12}, Rule: mustParse(10, "- record: up:count:4\n expr: count(up)\n"), }, { - State: discovery.Removed, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Removed, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{7}, Rule: mustParse(6, "- record: up:count:2\n expr: count(up)\n"), }, @@ -520,16 +540,20 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Modified, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{4}, Rule: mustParse(1, "- alert: rule1\n expr: sum(foo) by(job)\n for: 0s\n"), }, { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(4, "- alert: rule2\n expr: sum(foo) by(job)\n for: 0s\n"), }, @@ -556,16 +580,20 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(1, "- alert: rule2\n expr: sum(foo) by(job)\n"), }, { - State: discovery.Removed, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Removed, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{2, 3}, Rule: mustParse(1, "- alert: rule1\n expr: sum(foo) by(job)\n"), }, @@ -592,16 +620,20 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(1, "- alert: rule1\n expr: sum(foo) by(job)\n"), }, { - State: discovery.Removed, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Removed, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{4, 5}, Rule: mustParse(3, "- alert: rule2\n expr: sum(foo) by(job)\n"), }, @@ -632,23 +664,29 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(1, "- alert: rule1\n expr: sum(foo) by(job)\n"), }, { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(3, "- alert: rule3\n expr: sum(foo) by(job)\n"), }, { - State: discovery.Removed, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Removed, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{4, 5}, Rule: mustParse(3, "- alert: rule2\n expr: sum(foo) by(job)\n"), }, @@ -680,16 +718,20 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, nil), "main", 4), entries: []discovery.Entry{ { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: nil, Rule: mustParse(4, "- record: up:count\n expr: count(up)\n"), }, { - State: discovery.Removed, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Removed, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{3}, PathError: mustErr(` groups: @@ -729,30 +771,38 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(1, "- alert: rule1\n expr: sum(foo) by(job)\n"), }, { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(3, "- alert: rule2\n expr: sum(foo) by(job)\n"), }, { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{6, 7}, Rule: mustParse(5, "- alert: rule2\n expr: sum(foo) by(job)\n"), }, { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{8, 9}, Rule: mustParse(7, "- alert: rule1\n expr: sum(foo) by(job)\n"), }, @@ -783,31 +833,39 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Modified, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{3}, Rule: mustParse(1, "- alert: rule1\n expr: up == 0\n"), }, { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{4, 5}, Rule: mustParse(3, "- alert: rule1\n expr: up == 1\n"), }, { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{6, 7}, Rule: mustParse(5, "- alert: rule1\n expr: up != 0\n"), }, { - State: discovery.Added, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", - ModifiedLines: []int{8}, + State: discovery.Added, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, + ModifiedLines: []int{8, 9}, Rule: mustParse(7, "- alert: rule2\n expr: sum(foo) by(job)\n"), }, }, @@ -844,16 +902,20 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Excluded, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Excluded, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{}, Rule: mustParse(1, "- alert: rule1\n expr: sum(foo) by(job)\n for: 1s\n"), }, { - State: discovery.Modified, - ReportedPath: "rules.yml", - SourcePath: "rules.yml", + State: discovery.Modified, + Path: discovery.Path{ + Name: "rules.yml", + SymlinkTarget: "rules.yml", + }, ModifiedLines: []int{7, 8, 9, 10, 11, 12}, Rule: mustParse(4, "- alert: rule2\n expr: sum(foo) by(job)\n keep_firing_for: 5m\n for: 0s\n annotations:\n foo: bar\n labels:\n foo: bar\n"), }, @@ -878,9 +940,46 @@ groups: finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), entries: []discovery.Entry{ { - State: discovery.Moved, - ReportedPath: "b.yml", - SourcePath: "b.yml", + State: discovery.Moved, + Path: discovery.Path{ + Name: "b.yml", + SymlinkTarget: "b.yml", + }, + ModifiedLines: []int{1, 2, 3}, + Rule: mustParse(1, "- alert: rule\n expr: up == 0\n"), + }, + }, + }, + { + title: "rule modified then the file renamed", + setup: func(t *testing.T) { + commitFile(t, "a.yml", ` +- alert: rule + # pint disable promql/series + expr: up == 0 +`, "v1") + + _, err := git.RunGit("checkout", "-b", "v2") + require.NoError(t, err, "git checkout v2") + + commitFile(t, "a.yml", ` +- alert: rule + expr: up == 0 +`, "v2") + + _, err = git.RunGit("mv", "a.yml", "b.yml") + require.NoError(t, err, "git mv") + + gitCommit(t, "v3") + }, + finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, includeAll), "main", 4), + entries: []discovery.Entry{ + { + State: discovery.Moved, + Path: discovery.Path{ + Name: "b.yml", + SymlinkTarget: "b.yml", + }, ModifiedLines: []int{1, 2, 3}, Rule: mustParse(1, "- alert: rule\n expr: up == 0\n"), }, diff --git a/internal/discovery/glob_test.go b/internal/discovery/glob_test.go index c7ad3504..bba0ff3a 100644 --- a/internal/discovery/glob_test.go +++ b/internal/discovery/glob_test.go @@ -71,9 +71,11 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, []*regexp.Regexp{regexp.MustCompile(".*")})), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "bar.yml", - SourcePath: "bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "bar.yml", + SymlinkTarget: "bar.yml", + }, Rule: testRules[0], ModifiedLines: testRules[0].Lines.Expand(), Owner: "bob", @@ -85,9 +87,11 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, []*regexp.Regexp{regexp.MustCompile(".*")})), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "foo/bar.yml", - SourcePath: "foo/bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "foo/bar.yml", + SymlinkTarget: "foo/bar.yml", + }, Rule: testRules[0], ModifiedLines: testRules[0].Lines.Expand(), Owner: "alice", @@ -99,9 +103,11 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, nil)), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "bar.yml", - SourcePath: "bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "bar.yml", + SymlinkTarget: "bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", @@ -113,9 +119,11 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, []*regexp.Regexp{regexp.MustCompile(".*")})), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "bar.yml", - SourcePath: "bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "bar.yml", + SymlinkTarget: "bar.yml", + }, PathError: errors.New("yaml: line 2: mapping values are not allowed in this context"), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", @@ -128,17 +136,21 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, nil)), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "bar.yml", - SourcePath: "bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "bar.yml", + SymlinkTarget: "bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", }, { - State: discovery.Noop, - ReportedPath: "bar.yml", - SourcePath: "link.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "link.yml", + SymlinkTarget: "bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", @@ -154,25 +166,31 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, nil)), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "a/bar.yml", - SourcePath: "a/bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "a/bar.yml", + SymlinkTarget: "a/bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", }, { - State: discovery.Noop, - ReportedPath: "a/bar.yml", - SourcePath: "b/c/link.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "b/c/link.yml", + SymlinkTarget: "a/bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", }, { - State: discovery.Noop, - ReportedPath: "a/bar.yml", - SourcePath: "b/link.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "b/link.yml", + SymlinkTarget: "a/bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", @@ -203,17 +221,21 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, nil)), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "a/bar.yml", - SourcePath: "a/bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "a/bar.yml", + SymlinkTarget: "a/bar.yml", + }, PathError: parseErr("xxx:\nyyy:\n"), ModifiedLines: []int{1, 2}, Owner: "", }, { - State: discovery.Noop, - ReportedPath: "a/bar.yml", - SourcePath: "b/c/link.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "b/c/link.yml", + SymlinkTarget: "a/bar.yml", + }, PathError: parseErr("xxx:\nyyy:\n"), ModifiedLines: []int{1, 2}, Owner: "", @@ -242,17 +264,21 @@ func TestGlobPathFinder(t *testing.T) { finder: discovery.NewGlobFinder([]string{"*"}, git.NewPathFilter(nil, nil, nil)), entries: []discovery.Entry{ { - State: discovery.Noop, - ReportedPath: "a/bar.yml", - SourcePath: "a/bar.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "a/bar.yml", + SymlinkTarget: "a/bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", }, { - State: discovery.Noop, - ReportedPath: "a/bar.yml", - SourcePath: "b/c/link.yml", + State: discovery.Noop, + Path: discovery.Path{ + Name: "b/c/link.yml", + SymlinkTarget: "a/bar.yml", + }, PathError: parseErr(testRuleBody), ModifiedLines: []int{1, 2, 3, 4}, Owner: "bob", diff --git a/internal/discovery/symlinks.go b/internal/discovery/symlinks.go index 0c9b76e2..2354280e 100644 --- a/internal/discovery/symlinks.go +++ b/internal/discovery/symlinks.go @@ -61,17 +61,19 @@ func addSymlinkedEntries(entries []Entry) ([]Entry, error) { if entry.Rule.Error.Err != nil { continue } - if entry.SourcePath != entry.ReportedPath { + if entry.Path.Name != entry.Path.SymlinkTarget { continue } for _, sl := range slinks { - if sl.to == entry.SourcePath { + if sl.to == entry.Path.Name { slog.Debug("Found a symlink", slog.String("to", sl.to), slog.String("from", sl.from)) nentries = append(nentries, Entry{ - State: entry.State, - ReportedPath: sl.to, - SourcePath: sl.from, + State: entry.State, + Path: Path{ + Name: sl.from, + SymlinkTarget: sl.to, + }, ModifiedLines: entry.ModifiedLines, Rule: entry.Rule, Owner: entry.Owner, diff --git a/internal/git/changes.go b/internal/git/changes.go index bfc6c567..4423dd13 100644 --- a/internal/git/changes.go +++ b/internal/git/changes.go @@ -62,9 +62,10 @@ type PathDiff struct { } type FileChange struct { - Commits []string Path PathDiff Body BodyDiff + Commits []string + Status FileStatus } func Changes(cmd CommandRunner, cr CommitRangeResults, filter PathFilter) ([]*FileChange, error) { @@ -109,54 +110,96 @@ func Changes(cmd CommandRunner, cr CommitRangeResults, filter PathFilter) ([]*Fi continue } - change := getChangeByPath(changes, dstPath) - if change == nil { - beforeType := getTypeForPath(cmd, commit+"^", srcPath) - change = &FileChange{ - Path: PathDiff{ - Before: Path{ - Name: srcPath, - Type: beforeType, - SymlinkTarget: resolveSymlinkTarget(cmd, commit+"^", srcPath, beforeType), - }, - After: Path{ - Name: dstPath, - }, + // Rest is populated inside the next loop. + change := &FileChange{ + Status: status, + Path: PathDiff{ + After: Path{ + Name: dstPath, }, + }, + } + + prev := getChangeByPath(changes, srcPath) + slog.Debug("Looking for previous changes", + slog.String("src", srcPath), + slog.String("dst", dstPath), + slog.String("commit", commit), + ) + if prev != nil { + slog.Debug("Found a previous change", + slog.Any("commits", prev.Commits), + slog.String("status", string(prev.Status)), + slog.String("path", prev.Path.Before.Name), + slog.String("target", prev.Path.Before.SymlinkTarget), + slog.Any("type", prev.Path.Before.Type), + ) + change.Commits = append(change.Commits, prev.Commits...) + change.Path.Before = prev.Path.Before + // Remove any changes for "BEFORE" path we might already have + changes = changesWithout(changes, srcPath) + } else { + slog.Debug("No previous change found") + switch change.Status { + case FileAdded, FileCopied: + change.Path.Before.Name = "" + change.Path.Before.SymlinkTarget = "" + // If a path changed type we'll see A but we can still query for old type. + change.Path.Before.Type = getTypeForPath(cmd, commit+"^", srcPath) + if change.Path.Before.Type != Missing { + // If it was a type change then + change.Path.Before.Name = srcPath + change.Path.Before.Type = getTypeForPath(cmd, commit+"^", srcPath) + } + case FileDeleted, FileRenamed, FileModified, FileTypeChanged: + change.Path.Before.Name = srcPath + change.Path.Before.Type = getTypeForPath(cmd, commit+"^", srcPath) + change.Path.Before.SymlinkTarget = resolveSymlinkTarget(cmd, commit+"^", srcPath, change.Path.Before.Type) } - switch status { - case FileAdded: - // newly added file, there's no "BEFORE" version - case FileCopied: - // file copied from other location, there's no "BEFORE" version - case FileDeleted: - // delete file, there's no "AFTER" version - change.Body.Before = getContentAtCommit(cmd, commit+"^", change.Path.Before.SymlinkTarget) - case FileModified: - // modified file, there's both "BEFORE" and "AFTER" - change.Body.Before = getContentAtCommit(cmd, commit+"^", change.Path.Before.SymlinkTarget) - case FileRenamed: - // rename could be only partial so there's both "BEFORE" and "AFTER" - change.Body.Before = getContentAtCommit(cmd, commit+"^", change.Path.Before.SymlinkTarget) - case FileTypeChanged: - // type change, could be file -> dir or symlink -> file - // so there's both "BEFORE" and "AFTER" - change.Body.Before = getContentAtCommit(cmd, commit+"^", change.Path.Before.SymlinkTarget) - default: - slog.Debug("Unknown git change", slog.String("path", dstPath), slog.String("commit", commit), slog.String("change", parts[0])) - } - changes = append(changes, change) } + change.Commits = append(change.Commits, commit) + + changes = append(changes, change) } + slog.Debug("Parsed git log", slog.Int("changes", len(changes))) for _, change := range changes { + slog.Debug( + "File change", + slog.Any("commits", change.Commits), + slog.String("status", string(change.Status)), + slog.String("before", change.Path.Before.Name), + slog.String("after", change.Path.After.Name), + ) + + if change.Path.Before.Name != "" { + change.Path.Before.Type = getTypeForPath(cmd, change.Commits[0]+"^", change.Path.Before.Name) + change.Path.Before.SymlinkTarget = resolveSymlinkTarget(cmd, change.Commits[0]+"^", change.Path.Before.Name, change.Path.Before.Type) + change.Body.Before = getContentAtCommit(cmd, change.Commits[0]+"^", change.Path.Before.EffectivePath()) + } + lastCommit := change.Commits[len(change.Commits)-1] + if change.Path.After.Name != "" && change.Status != FileDeleted { + change.Path.After.Type = getTypeForPath(cmd, lastCommit, change.Path.After.Name) + change.Path.After.SymlinkTarget = resolveSymlinkTarget(cmd, lastCommit, change.Path.After.Name, change.Path.After.Type) + change.Body.After = getContentAtCommit(cmd, lastCommit, change.Path.After.EffectivePath()) + } - change.Path.After.Type = getTypeForPath(cmd, lastCommit, change.Path.After.Name) - change.Path.After.SymlinkTarget = resolveSymlinkTarget(cmd, lastCommit, change.Path.After.Name, change.Path.After.Type) - change.Body.After = getContentAtCommit(cmd, lastCommit, change.Path.After.EffectivePath()) + slog.Debug( + "Updated file change", + slog.Any("commits", change.Commits), + slog.String("before.path", change.Path.Before.Name), + slog.String("before.target", change.Path.Before.SymlinkTarget), + slog.Any("before.type", change.Path.Before.Type), + slog.String("before.body", string(change.Body.Before)), + slog.String("after.path", change.Path.After.Name), + slog.String("after.target", change.Path.After.SymlinkTarget), + slog.Any("after.type", change.Path.After.Type), + slog.String("after.body", string(change.Body.After)), + slog.Any("modifiedLines", change.Body.ModifiedLines), + ) switch { case change.Path.Before.Type != Missing && change.Path.After.Type == Symlink: @@ -176,8 +219,11 @@ func Changes(cmd CommandRunner, cr CommitRangeResults, filter PathFilter) ([]*Fi case change.Path.Before.Type != Missing && change.Path.After.Type == Missing: // new file body is empty, meaning that every line was modified change.Body.ModifiedLines = CountLines(change.Body.Before) + case change.Path.Before.Type == Missing && change.Path.After.Type == Missing: + // file was added and then removed + change.Body.ModifiedLines = []int{} default: - slog.Debug("Unhandled change", slog.String("change", fmt.Sprintf("+%v", change))) + slog.Warn("Unhandled change", slog.String("change", fmt.Sprintf("+%v", change))) } if change.Path.Before.Name == change.Path.Before.SymlinkTarget { @@ -191,6 +237,12 @@ func Changes(cmd CommandRunner, cr CommitRangeResults, filter PathFilter) ([]*Fi return changes, nil } +func changesWithout(changes []*FileChange, fpath string) []*FileChange { + return slices.DeleteFunc(changes, func(e *FileChange) bool { + return e.Path.After.Name == fpath + }) +} + func getChangeByPath(changes []*FileChange, fpath string) *FileChange { for _, c := range changes { if c.Path.After.Name == fpath { @@ -201,7 +253,10 @@ func getChangeByPath(changes []*FileChange, fpath string) *FileChange { } func getModifiedLines(cmd CommandRunner, commits []string, fpath, atCommit string) ([]int, error) { - slog.Debug("Getting list of modified lines", slog.String("commits", fmt.Sprint(commits)), slog.String("path", fpath)) + slog.Debug("Getting list of modified lines", + slog.Any("commits", commits), + slog.String("path", fpath), + ) lines, err := Blame(cmd, fpath, atCommit) if err != nil { return nil, err @@ -209,7 +264,7 @@ func getModifiedLines(cmd CommandRunner, commits []string, fpath, atCommit strin modLines := make([]int, 0, len(lines)) for _, line := range lines { - if !slices.Contains(commits, line.Commit) { + if !slices.Contains(commits, line.Commit) && line.Line == line.PrevLine { continue } modLines = append(modLines, line.Line) diff --git a/internal/git/changes_test.go b/internal/git/changes_test.go index 38b11b23..43339b58 100644 --- a/internal/git/changes_test.go +++ b/internal/git/changes_test.go @@ -80,6 +80,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "index.txt", @@ -120,6 +121,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "index.txt", @@ -137,6 +139,7 @@ func TestChanges(t *testing.T) { }, }, { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "index.txt/.keep", @@ -157,7 +160,7 @@ func TestChanges(t *testing.T) { err: "", }, { - title: "add file, delete and re-add", + title: "delete and re-add", setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo"), 0o644)) @@ -178,6 +181,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1", "2"}, Path: git.PathDiff{ Before: git.Path{ Name: "index.txt", @@ -219,6 +223,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "second file.txt", @@ -259,6 +264,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "index.txt", @@ -299,6 +305,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1", "2"}, Path: git.PathDiff{ Before: git.Path{ Name: "index.txt", @@ -339,9 +346,10 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ - Name: "second.txt", + Name: "", Type: git.Missing, }, After: git.Path{ @@ -355,9 +363,10 @@ func TestChanges(t *testing.T) { }, }, { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ - Name: "third.txt", + Name: "", Type: git.Missing, }, After: git.Path{ @@ -394,6 +403,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "second.txt", @@ -433,6 +443,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "second.txt", @@ -475,6 +486,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "dir/first.txt", @@ -492,6 +504,7 @@ func TestChanges(t *testing.T) { }, }, { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "dir/second.txt", @@ -537,6 +550,7 @@ func TestChanges(t *testing.T) { }, changes: []*git.FileChange{ { + Commits: []string{"1"}, Path: git.PathDiff{ Before: git.Path{ Name: "dir/second.txt", @@ -558,6 +572,48 @@ func TestChanges(t *testing.T) { }, err: "", }, + { + title: "rule modified then file renamed", + setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + mustRun(t, "init", "--initial-branch=main", ".") + require.NoError(t, os.WriteFile("main.txt", []byte("l1\nl2\nl3\n"), 0o644)) + mustRun(t, "add", "main.txt") + gitCommit(t, "init") + + mustRun(t, "checkout", "-b", "v2") + require.NoError(t, os.WriteFile("main.txt", []byte("l1\nl3\n"), 0o644)) + mustRun(t, "add", "main.txt") + gitCommit(t, "edit") + + mustRun(t, "mv", "main.txt", "pr.txt") + gitCommit(t, "rename") + + cr, err := git.CommitRange(debugGitRun(t), "main") + require.NoError(t, err) + return debugGitRun(t), cr + }, + changes: []*git.FileChange{ + { + Commits: []string{"1", "2"}, + Path: git.PathDiff{ + Before: git.Path{ + Name: "main.txt", + Type: git.File, + }, + After: git.Path{ + Name: "pr.txt", + Type: git.File, + }, + }, + Body: git.BodyDiff{ + Before: []byte("l1\nl2\nl3\n"), + After: []byte("l1\nl3\n"), + ModifiedLines: []int{2}, + }, + }, + }, + err: "", + }, } for _, tc := range testCases { @@ -575,11 +631,12 @@ func TestChanges(t *testing.T) { require.Nil(t, changes) } else { require.NoError(t, err) - require.Len(t, changes, len(tc.changes)) for i := range tc.changes { + require.Len(t, changes[i].Commits, len(tc.changes[i].Commits), "changes[%d].Commits", i) require.Equal(t, tc.changes[i].Path, changes[i].Path, "changes[%d].Path", i) require.Equal(t, tc.changes[i].Body, changes[i].Body, "changes[%d].Body", i) } + require.Len(t, changes, len(tc.changes)) } }) } diff --git a/internal/git/git.go b/internal/git/git.go index 2955c032..ec2a675b 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -14,6 +14,7 @@ import ( type LineBlame struct { Filename string Commit string + PrevLine int Line int } @@ -66,17 +67,18 @@ func Blame(cmd CommandRunner, path, commit string) (lines LineBlames, err error) case strings.HasPrefix(line, "boundary"): continue case strings.HasPrefix(line, "\t"): - if cl.Filename == path { - lines = append(lines, cl) - } + lines = append(lines, cl) + cl.PrevLine = 0 default: parts := strings.Split(line, " ") if len(parts) < 3 { return nil, fmt.Errorf("failed to parse line number from line: %q", line) } cl.Commit = parts[0] - cl.Line, err = strconv.Atoi(parts[2]) - if err != nil { + if cl.PrevLine, err = strconv.Atoi(parts[1]); err != nil { + return nil, fmt.Errorf("failed to parse line number from %q: %w", line, err) + } + if cl.Line, err = strconv.Atoi(parts[2]); err != nil { return nil, fmt.Errorf("failed to parse line number from %q: %w", line, err) } } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 6e6ba876..0bd19b14 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -13,7 +13,7 @@ import ( "github.com/cloudflare/pint/internal/git" ) -func blameLine(sha string, line int, filename, content string) string { +func blameLine(sha string, line, prevLine int, filename, content string) string { return fmt.Sprintf(`%s %d %d 1 author Alice Mock author-mail @@ -27,7 +27,7 @@ summary Mock commit title boundary filename %s %s -`, sha, line, line, filename, content) +`, sha, prevLine, line, filename, content) } func TestGitBlame(t *testing.T) { @@ -54,7 +54,7 @@ func TestGitBlame(t *testing.T) { }, { mock: func(_ ...string) ([]byte, error) { - content := blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 1, "foo.txt", "") + content := blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 1, 1, "foo.txt", "") return []byte(content), nil }, path: "foo.txt", @@ -62,16 +62,14 @@ func TestGitBlame(t *testing.T) { { Filename: "foo.txt", Line: 1, + PrevLine: 1, Commit: "b33a88cea35abc47f9973983626e1c6f3f3abc44", }, }, }, { mock: func(_ ...string) ([]byte, error) { - content := blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 1, "foo.txt", "") + - blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 2, "foo.txt", "") + - blameLine("82987dec74ba8e434ba393d83491ace784473291", 3, "foo.txt", "") + - blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 4, "bar.txt", "") + content := blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 1, 5, "foo.txt", "") return []byte(content), nil }, path: "foo.txt", @@ -79,10 +77,45 @@ func TestGitBlame(t *testing.T) { { Filename: "foo.txt", Line: 1, + PrevLine: 5, + Commit: "b33a88cea35abc47f9973983626e1c6f3f3abc44", + }, + }, + }, + { + mock: func(_ ...string) ([]byte, error) { + content := blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 1, 1, "foo.txt", "") + + blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 2, 2, "foo.txt", "") + + blameLine("82987dec74ba8e434ba393d83491ace784473291", 3, 3, "foo.txt", "") + + blameLine("b33a88cea35abc47f9973983626e1c6f3f3abc44", 4, 4, "bar.txt", "") + return []byte(content), nil + }, + path: "foo.txt", + output: git.LineBlames{ + { + Filename: "foo.txt", + Line: 1, + PrevLine: 1, + Commit: "b33a88cea35abc47f9973983626e1c6f3f3abc44", + }, + { + Filename: "foo.txt", + Line: 2, + PrevLine: 2, + Commit: "b33a88cea35abc47f9973983626e1c6f3f3abc44", + }, + { + Filename: "foo.txt", + Line: 3, + PrevLine: 3, + Commit: "82987dec74ba8e434ba393d83491ace784473291", + }, + { + Filename: "bar.txt", + Line: 4, + PrevLine: 4, Commit: "b33a88cea35abc47f9973983626e1c6f3f3abc44", }, - {Filename: "foo.txt", Line: 2, Commit: "b33a88cea35abc47f9973983626e1c6f3f3abc44"}, - {Filename: "foo.txt", Line: 3, Commit: "82987dec74ba8e434ba393d83491ace784473291"}, }, }, } diff --git a/internal/reporter/bitbucket_api.go b/internal/reporter/bitbucket_api.go index 39fe21ab..e4c9d239 100644 --- a/internal/reporter/bitbucket_api.go +++ b/internal/reporter/bitbucket_api.go @@ -591,7 +591,7 @@ func (bb bitBucketAPI) makeComments(summary Summary, changes *bitBucketPRChanges var buf strings.Builder comments := []BitBucketPendingComment{} for _, reports := range dedupReports(summary.reports) { - if _, ok := changes.pathModifiedLines[reports[0].ReportedPath]; !ok { + if _, ok := changes.pathModifiedLines[reports[0].Path.SymlinkTarget]; !ok { continue } @@ -613,10 +613,10 @@ func (bb bitBucketAPI) makeComments(summary Summary, changes *bitBucketPRChanges buf.WriteString(report.Problem.Details) buf.WriteString("\n\n") } - if report.ReportedPath != report.SourcePath { + if report.Path.SymlinkTarget != report.Path.Name { buf.WriteString(":leftwards_arrow_with_hook: This problem was detected on a symlinked file ") buf.WriteRune('`') - buf.WriteString(report.SourcePath) + buf.WriteString(report.Path.Name) buf.WriteString("`.\n\n") } } @@ -641,7 +641,7 @@ func (bb bitBucketAPI) makeComments(summary Summary, changes *bitBucketPRChanges pending := pendingComment{ severity: severity, - path: reports[0].ReportedPath, + path: reports[0].Path.SymlinkTarget, line: reports[0].Problem.Lines.Last, text: buf.String(), anchor: reports[0].Problem.Anchor, @@ -836,11 +836,11 @@ func reportToAnnotation(report Report) BitBucketAnnotation { if reportLine != srcLine { msgPrefix = fmt.Sprintf("Problem reported on unmodified line %d, annotation moved here: ", srcLine) } - if report.ReportedPath != report.SourcePath { + if report.Path.SymlinkTarget != report.Path.Name { if msgPrefix == "" { - msgPrefix = fmt.Sprintf("Problem detected on symlinked file %s: ", report.SourcePath) + msgPrefix = fmt.Sprintf("Problem detected on symlinked file %s: ", report.Path.Name) } else { - msgPrefix = fmt.Sprintf("Problem detected on symlinked file %s. %s", report.SourcePath, msgPrefix) + msgPrefix = fmt.Sprintf("Problem detected on symlinked file %s. %s", report.Path.Name, msgPrefix) } } @@ -857,7 +857,7 @@ func reportToAnnotation(report Report) BitBucketAnnotation { } return BitBucketAnnotation{ - Path: report.ReportedPath, + Path: report.Path.SymlinkTarget, Line: reportLine, Message: fmt.Sprintf("%s%s: %s", msgPrefix, report.Problem.Reporter, report.Problem.Text), Severity: severity, @@ -876,7 +876,7 @@ func dedupReports(src []Report) (dst [][]Report) { if d[0].Problem.Reporter != report.Problem.Reporter { continue } - if d[0].ReportedPath != report.ReportedPath { + if d[0].Path.SymlinkTarget != report.Path.SymlinkTarget { continue } if d[0].Problem.Lines.First != report.Problem.Lines.First { diff --git a/internal/reporter/bitbucket_api_test.go b/internal/reporter/bitbucket_api_test.go index 54fa52a2..442f5f7e 100644 --- a/internal/reporter/bitbucket_api_test.go +++ b/internal/reporter/bitbucket_api_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/cloudflare/pint/internal/checks" + "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/parser" ) @@ -111,8 +112,10 @@ func TestReportToAnnotation(t *testing.T) { { description: "fatal report on modified line", input: Report{ - ReportedPath: "foo.yaml", - SourcePath: "foo.yaml", + Path: discovery.Path{ + SymlinkTarget: "foo.yaml", + Name: "foo.yaml", + }, ModifiedLines: []int{4, 5, 6}, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -137,8 +140,10 @@ func TestReportToAnnotation(t *testing.T) { { description: "bug report on modified line", input: Report{ - ReportedPath: "foo.yaml", - SourcePath: "foo.yaml", + Path: discovery.Path{ + SymlinkTarget: "foo.yaml", + Name: "foo.yaml", + }, ModifiedLines: []int{4, 5, 6}, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -162,8 +167,10 @@ func TestReportToAnnotation(t *testing.T) { { description: "warning report on modified line", input: Report{ - ReportedPath: "foo.yaml", - SourcePath: "foo.yaml", + Path: discovery.Path{ + SymlinkTarget: "foo.yaml", + Name: "foo.yaml", + }, ModifiedLines: []int{4, 5, 6}, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -187,8 +194,10 @@ func TestReportToAnnotation(t *testing.T) { { description: "information report on modified line", input: Report{ - ReportedPath: "foo.yaml", - SourcePath: "foo.yaml", + Path: discovery.Path{ + SymlinkTarget: "foo.yaml", + Name: "foo.yaml", + }, ModifiedLines: []int{4, 5, 6}, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -212,8 +221,10 @@ func TestReportToAnnotation(t *testing.T) { { description: "fatal report on symlinked file", input: Report{ - ReportedPath: "foo.yaml", - SourcePath: "bar.yaml", + Path: discovery.Path{ + SymlinkTarget: "foo.yaml", + Name: "bar.yaml", + }, ModifiedLines: []int{4, 5, 6}, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -237,8 +248,10 @@ func TestReportToAnnotation(t *testing.T) { { description: "fatal report on symlinked file on unmodified line", input: Report{ - ReportedPath: "foo.yaml", - SourcePath: "bar.yaml", + Path: discovery.Path{ + SymlinkTarget: "foo.yaml", + Name: "bar.yaml", + }, ModifiedLines: []int{4, 5, 6}, Problem: checks.Problem{ Lines: parser.LineRange{ @@ -262,8 +275,10 @@ func TestReportToAnnotation(t *testing.T) { { description: "information report on unmodified line", input: Report{ - ReportedPath: "foo.yaml", - SourcePath: "foo.yaml", + Path: discovery.Path{ + SymlinkTarget: "foo.yaml", + Name: "foo.yaml", + }, ModifiedLines: []int{4, 5, 6}, Problem: checks.Problem{ Lines: parser.LineRange{ diff --git a/internal/reporter/bitbucket_comments_test.go b/internal/reporter/bitbucket_comments_test.go index ba410409..e73ebb7f 100644 --- a/internal/reporter/bitbucket_comments_test.go +++ b/internal/reporter/bitbucket_comments_test.go @@ -10,6 +10,7 @@ import ( "github.com/neilotoole/slogt" "github.com/cloudflare/pint/internal/checks" + "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/parser" ) @@ -40,8 +41,10 @@ func TestBitBucketMakeComments(t *testing.T) { maxComments: 50, summary: Summary{reports: []Report{ { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -56,8 +59,10 @@ func TestBitBucketMakeComments(t *testing.T) { maxComments: 50, summary: Summary{reports: []Report{ { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -71,8 +76,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Warning, @@ -86,8 +93,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -101,8 +110,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "symlink.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "symlink.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -116,8 +127,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "second.yaml", - SourcePath: "second.yaml", + Path: discovery.Path{ + SymlinkTarget: "second.yaml", + Name: "second.yaml", + }, ModifiedLines: []int{1, 2, 3}, Problem: checks.Problem{ Anchor: checks.AnchorBefore, @@ -132,8 +145,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "second.yaml", - SourcePath: "second.yaml", + Path: discovery.Path{ + SymlinkTarget: "second.yaml", + Name: "second.yaml", + }, ModifiedLines: []int{1, 2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -220,8 +235,10 @@ func TestBitBucketMakeComments(t *testing.T) { maxComments: 50, summary: Summary{reports: []Report{ { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -235,8 +252,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -277,8 +296,10 @@ func TestBitBucketMakeComments(t *testing.T) { maxComments: 50, summary: Summary{reports: []Report{ { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -292,8 +313,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -334,8 +357,10 @@ func TestBitBucketMakeComments(t *testing.T) { maxComments: 50, summary: Summary{reports: []Report{ { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -349,8 +374,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -391,8 +418,10 @@ func TestBitBucketMakeComments(t *testing.T) { maxComments: 2, summary: Summary{reports: []Report{ { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -406,8 +435,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Warning, @@ -421,8 +452,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "rule.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "rule.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -436,8 +469,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "rule.yaml", - SourcePath: "symlink.yaml", + Path: discovery.Path{ + SymlinkTarget: "rule.yaml", + Name: "symlink.yaml", + }, ModifiedLines: []int{2, 3}, Problem: checks.Problem{ Severity: checks.Bug, @@ -451,8 +486,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "second.yaml", - SourcePath: "second.yaml", + Path: discovery.Path{ + SymlinkTarget: "second.yaml", + Name: "second.yaml", + }, ModifiedLines: []int{1, 2, 3}, Problem: checks.Problem{ Anchor: checks.AnchorBefore, @@ -467,8 +504,10 @@ func TestBitBucketMakeComments(t *testing.T) { }, }, { - ReportedPath: "second.yaml", - SourcePath: "second.yaml", + Path: discovery.Path{ + SymlinkTarget: "second.yaml", + Name: "second.yaml", + }, ModifiedLines: []int{1, 2, 3}, Problem: checks.Problem{ Severity: checks.Bug, diff --git a/internal/reporter/bitbucket_test.go b/internal/reporter/bitbucket_test.go index d943abd8..d73e9475 100644 --- a/internal/reporter/bitbucket_test.go +++ b/internal/reporter/bitbucket_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" "github.com/cloudflare/pint/internal/checks" + "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/git" "github.com/cloudflare/pint/internal/parser" "github.com/cloudflare/pint/internal/reporter" @@ -113,8 +114,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2}, Rule: mockRules[0], Problem: checks.Problem{}, @@ -137,8 +140,10 @@ func TestBitBucketReporter(t *testing.T) { report: emptyReport, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2}, Rule: mockRules[0], Problem: checks.Problem{}, @@ -162,8 +167,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2}, Rule: mockRules[0], Problem: checks.Problem{}, @@ -236,8 +243,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -251,8 +260,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "bar.txt", - SourcePath: "bar.txt", + Path: discovery.Path{ + SymlinkTarget: "bar.txt", + Name: "bar.txt", + }, ModifiedLines: []int{}, Rule: mockRules[1], Problem: checks.Problem{ @@ -266,8 +277,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -281,8 +294,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[0], Problem: checks.Problem{ @@ -296,8 +311,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -502,8 +519,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -517,8 +536,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -532,8 +553,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[0], Problem: checks.Problem{ @@ -547,8 +570,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -625,8 +650,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{3, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -690,8 +717,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, Rule: mockRules[1], ModifiedLines: []int{2, 4}, Problem: checks.Problem{ @@ -705,8 +734,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, Rule: mockRules[1], ModifiedLines: []int{2, 4}, Problem: checks.Problem{ @@ -720,8 +751,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, Rule: mockRules[0], ModifiedLines: []int{2, 4}, Problem: checks.Problem{ @@ -735,8 +768,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, Rule: mockRules[1], ModifiedLines: []int{2, 4}, Problem: checks.Problem{ @@ -853,8 +888,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -868,8 +905,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -883,8 +922,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[0], Problem: checks.Problem{ @@ -899,8 +940,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "symlink.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "symlink.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -1421,8 +1464,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "index.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "index.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -1582,8 +1627,10 @@ func TestBitBucketReporter(t *testing.T) { gitCmd: fakeGit, reports: []reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -1597,8 +1644,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -1612,8 +1661,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ @@ -1628,8 +1679,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[0], Problem: checks.Problem{ @@ -1644,8 +1697,10 @@ func TestBitBucketReporter(t *testing.T) { }, }, { - ReportedPath: "foo.txt", - SourcePath: "symlink.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "symlink.txt", + }, ModifiedLines: []int{2, 4}, Rule: mockRules[1], Problem: checks.Problem{ diff --git a/internal/reporter/console.go b/internal/reporter/console.go index bdd07461..4a1b9504 100644 --- a/internal/reporter/console.go +++ b/internal/reporter/console.go @@ -25,10 +25,10 @@ type ConsoleReporter struct { func (cr ConsoleReporter) Submit(summary Summary) (err error) { reports := summary.Reports() sort.Slice(reports, func(i, j int) bool { - if reports[i].SourcePath < reports[j].SourcePath { + if reports[i].Path.Name < reports[j].Path.Name { return true } - if reports[i].SourcePath > reports[j].SourcePath { + if reports[i].Path.Name > reports[j].Path.Name { return false } if reports[i].Problem.Lines.First < reports[j].Problem.Lines.First { @@ -52,21 +52,21 @@ func (cr ConsoleReporter) Submit(summary Summary) (err error) { continue } - if _, ok := perFile[report.SourcePath]; !ok { - perFile[report.SourcePath] = []string{} + if _, ok := perFile[report.Path.Name]; !ok { + perFile[report.Path.Name] = []string{} } var content string if report.Problem.Anchor == checks.AnchorAfter { - content, err = readFile(report.SourcePath) + content, err = readFile(report.Path.Name) if err != nil { return err } } - path := report.SourcePath - if report.SourcePath != report.ReportedPath { - path = fmt.Sprintf("%s ~> %s", report.SourcePath, report.ReportedPath) + path := report.Path.Name + if report.Path.Name != report.Path.SymlinkTarget { + path = fmt.Sprintf("%s ~> %s", report.Path.Name, report.Path.SymlinkTarget) } path = color.CyanString("%s:%s", path, report.Problem.Lines) if report.Problem.Anchor == checks.AnchorBefore { @@ -92,7 +92,7 @@ func (cr ConsoleReporter) Submit(summary Summary) (err error) { lastLine = len(lines) - 1 slog.Warn( "Tried to read more lines than present in the source file, this is likely due to '\n' usage in some rules, see https://github.com/cloudflare/pint/issues/20 for details", - slog.String("path", report.SourcePath), + slog.String("path", report.Path.Name), ) } @@ -102,7 +102,7 @@ func (cr ConsoleReporter) Submit(summary Summary) (err error) { } } - perFile[report.SourcePath] = append(perFile[report.SourcePath], strings.Join(msg, "")) + perFile[report.Path.Name] = append(perFile[report.Path.Name], strings.Join(msg, "")) } paths := []string{} diff --git a/internal/reporter/github.go b/internal/reporter/github.go index 666ebb9d..6154e448 100644 --- a/internal/reporter/github.go +++ b/internal/reporter/github.go @@ -319,7 +319,7 @@ func reportToGitHubComment(headCommit string, rep Report) *github.PullRequestCom c := github.PullRequestComment{ CommitID: github.String(headCommit), - Path: github.String(rep.ReportedPath), + Path: github.String(rep.Path.SymlinkTarget), Body: github.String(fmt.Sprintf( "%s [%s](https://cloudflare.github.io/pint/checks/%s.html): %s%s%s", problemIcon(rep.Problem.Severity), diff --git a/internal/reporter/github_test.go b/internal/reporter/github_test.go index ccff8130..e295c8fc 100644 --- a/internal/reporter/github_test.go +++ b/internal/reporter/github_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "github.com/cloudflare/pint/internal/checks" + "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/git" "github.com/cloudflare/pint/internal/parser" "github.com/cloudflare/pint/internal/reporter" @@ -79,7 +80,11 @@ filename %s }, reports: []reporter.Report{ { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "$1", + SymlinkTarget: "$1", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -125,7 +130,11 @@ filename %s }, reports: []reporter.Report{ { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -161,7 +170,11 @@ filename %s }, reports: []reporter.Report{ { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -208,7 +221,11 @@ filename %s }, reports: []reporter.Report{ { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -259,7 +276,11 @@ filename %s }, reports: []reporter.Report{ { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -306,7 +327,11 @@ filename %s }), reports: []reporter.Report{ { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -353,8 +378,8 @@ filename %s body, _ := io.ReadAll(r.Body) b := strings.TrimSpace(strings.TrimRight(string(body), "\n\t\r")) switch b { - case `{"body":":stop_sign: [mock1](https://cloudflare.github.io/pint/checks/mock1.html): syntax error1\n\nsyntax details1","path":"","line":2,"side":"RIGHT","commit_id":"fake-commit-id"}`: - case `{"body":":stop_sign: [mock2](https://cloudflare.github.io/pint/checks/mock2.html): syntax error2\n\nsyntax details2","path":"","line":2,"side":"RIGHT","commit_id":"fake-commit-id"}`: + case `{"body":":stop_sign: [mock1](https://cloudflare.github.io/pint/checks/mock1.html): syntax error1\n\nsyntax details1","path":"foo.txt","line":2,"side":"RIGHT","commit_id":"fake-commit-id"}`: + case `{"body":":stop_sign: [mock2](https://cloudflare.github.io/pint/checks/mock2.html): syntax error2\n\nsyntax details2","path":"foo.txt","line":2,"side":"RIGHT","commit_id":"fake-commit-id"}`: case `{"body":"This pint run would create 4 comment(s), which is more than 2 limit configured for pint.\n2 comments were skipped and won't be visibile on this PR."}`: default: t.Errorf("Unexpected comment: %s", b) @@ -364,7 +389,11 @@ filename %s }), reports: []reporter.Report{ { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -379,7 +408,11 @@ filename %s }, }, { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -394,7 +427,11 @@ filename %s }, }, { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ @@ -409,7 +446,11 @@ filename %s }, }, { - SourcePath: "foo.txt", + Path: discovery.Path{ + Name: "foo.txt", + SymlinkTarget: "foo.txt", + }, + ModifiedLines: []int{2}, Rule: mockRules[1], Problem: checks.Problem{ diff --git a/internal/reporter/reporter.go b/internal/reporter/reporter.go index c72c3bc8..8d0d44ef 100644 --- a/internal/reporter/reporter.go +++ b/internal/reporter/reporter.go @@ -5,12 +5,12 @@ import ( "time" "github.com/cloudflare/pint/internal/checks" + "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/parser" ) type Report struct { - ReportedPath string - SourcePath string + Path discovery.Path Owner string ModifiedLines []int Rule parser.Rule @@ -18,10 +18,10 @@ type Report struct { } func (r Report) isEqual(nr Report) bool { - if nr.ReportedPath != r.ReportedPath { + if nr.Path.SymlinkTarget != r.Path.SymlinkTarget { return false } - if nr.SourcePath != r.SourcePath { + if nr.Path.Name != r.Path.Name { return false } if nr.Owner != r.Owner { @@ -80,11 +80,11 @@ func (s Summary) hasReport(r Report) bool { func (s *Summary) SortReports() { sort.SliceStable(s.reports, func(i, j int) bool { - if s.reports[i].ReportedPath != s.reports[j].ReportedPath { - return s.reports[i].ReportedPath < s.reports[j].ReportedPath + if s.reports[i].Path.SymlinkTarget != s.reports[j].Path.SymlinkTarget { + return s.reports[i].Path.SymlinkTarget < s.reports[j].Path.SymlinkTarget } - if s.reports[i].SourcePath != s.reports[j].SourcePath { - return s.reports[i].SourcePath < s.reports[j].SourcePath + if s.reports[i].Path.Name != s.reports[j].Path.Name { + return s.reports[i].Path.Name < s.reports[j].Path.Name } if s.reports[i].Problem.Lines.First != s.reports[j].Problem.Lines.First { return s.reports[i].Problem.Lines.First < s.reports[j].Problem.Lines.First diff --git a/internal/reporter/teamcity.go b/internal/reporter/teamcity.go index e6b1190f..351e6888 100644 --- a/internal/reporter/teamcity.go +++ b/internal/reporter/teamcity.go @@ -30,7 +30,7 @@ type TeamCityReporter struct { } func (tc TeamCityReporter) name(report Report) string { - return fmt.Sprintf("%s:%d", report.ReportedPath, report.Problem.Lines.First) + return fmt.Sprintf("%s:%d", report.Path.SymlinkTarget, report.Problem.Lines.First) } func (tc TeamCityReporter) escape(s string) string { @@ -67,7 +67,7 @@ func (tc TeamCityReporter) Submit(summary Summary) error { } buf.WriteString("##teamcity[testFinished name='") - buf.WriteString(report.ReportedPath) + buf.WriteString(report.Path.SymlinkTarget) buf.WriteRune(':') buf.WriteString(strconv.Itoa(report.Problem.Lines.First)) buf.WriteString("']\n") diff --git a/internal/reporter/teamcity_test.go b/internal/reporter/teamcity_test.go index a7e07e14..33ad2360 100644 --- a/internal/reporter/teamcity_test.go +++ b/internal/reporter/teamcity_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/cloudflare/pint/internal/checks" + "github.com/cloudflare/pint/internal/discovery" "github.com/cloudflare/pint/internal/parser" "github.com/cloudflare/pint/internal/reporter" ) @@ -37,8 +38,10 @@ func TestTeamCityReporter(t *testing.T) { description: "info report", summary: reporter.NewSummary([]reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4, 5}, Rule: mockRules[0], Problem: checks.Problem{ @@ -66,8 +69,10 @@ func TestTeamCityReporter(t *testing.T) { description: "bug report", summary: reporter.NewSummary([]reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4, 5}, Rule: mockRules[0], Problem: checks.Problem{ @@ -94,8 +99,10 @@ func TestTeamCityReporter(t *testing.T) { description: "escaping characters", summary: reporter.NewSummary([]reporter.Report{ { - ReportedPath: "foo.txt", - SourcePath: "foo.txt", + Path: discovery.Path{ + SymlinkTarget: "foo.txt", + Name: "foo.txt", + }, ModifiedLines: []int{2, 4, 5}, Rule: mockRules[0], Problem: checks.Problem{