Skip to content

fix: fix analysis and test runners#151

Merged
sanket-deepsource merged 4 commits intomasterfrom
fix-runner
Feb 28, 2025
Merged

fix: fix analysis and test runners#151
sanket-deepsource merged 4 commits intomasterfrom
fix-runner

Conversation

@sourya-deepsource
Copy link
Contributor

@sourya-deepsource sourya-deepsource commented Feb 28, 2025

Description

This PR fixes

  1. The analysis runner:
  • When n files of a language existed in the same dir, the runner raised each issue n times.
  1. The test runner:
  • Filter out test files with no analyzer supporting them, as they are most likely YAML rule tests

Type of change

  • Bug fix
  • New feature/enhancement
  • New checker
  • Documentation update
  • Other (please describe)

Checklist

  • I have updated the documentation, if applicable
  • All new checkers are in the checkers directory
  • I have added tests, if applicable
  • I have tested my changes (run make testall)
  • Checker YAML files follow the required format
  • CI checks are passing

Related Issues

Additional Notes

Signed-off-by: Sourya Vatsyayan <sourya@deepsource.io>
Signed-off-by: Sourya Vatsyayan <sourya@deepsource.io>
Signed-off-by: Sourya Vatsyayan <sourya@deepsource.io>
Copy link
Contributor

@sanket-deepsource sanket-deepsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested some renames.

}

func RunAnalyzers(path string, analyzers []*Analyzer) ([]*Issue, error) {
func RunAnalyzers(path string, analyzers []*Analyzer, fileFilter func(string) bool) ([]*Issue, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the Analyzer refers to a checker here, can we rename this to RunCheckers?

func LoadYamlRules() (map[analysis.Language][]analysis.YmlRule, error) {
rulesMap := make(map[analysis.Language][]analysis.YmlRule)
err := fs.WalkDir(builtinCheckers, ".", func(path string, d fs.DirEntry, err error) error {
func ymlRuleFinder(rulesMap map[analysis.Language][]analysis.YmlRule) func(path string, d fs.DirEntry, err error) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The functions name should be an active verb. Can we rename this?

ana.issuesRaised = append(ana.issuesRaised, issue)
}

func RunYmlRules(path string, analyzers []*Analyzer) ([]*Issue, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using the term checkers. Can we rename this?


checkers := cmd.String("checkers")
if checkers == "local" {
return c.RunLints(patternRules, false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename this to RunCheckers?

Signed-off-by: Sourya Vatsyayan <sourya@deepsource.io>
@sanket-deepsource sanket-deepsource merged commit f4a5dfd into master Feb 28, 2025
1 check passed
@sanket-deepsource sanket-deepsource deleted the fix-runner branch February 28, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants