Skip to content

Commit

Permalink
Add support for including and excluding issue codes (#20)
Browse files Browse the repository at this point in the history
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
Co-authored-by: Tushar Sadhwani <[email protected]>
  • Loading branch information
3 people authored Mar 10, 2023
1 parent d364e0f commit b234906
Show file tree
Hide file tree
Showing 14 changed files with 361 additions and 22 deletions.
2 changes: 1 addition & 1 deletion cmd/scatr/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/spf13/cobra"
)

const version = "0.3.4"
const version = "0.4.0"

var versionCmd = &cobra.Command{
Use: "version",
Expand Down
62 changes: 61 additions & 1 deletion pragma/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,49 @@ import (
"bytes"
"io"
"log"
"path/filepath"
"strings"
)

type CheckMode int

const (
CheckAll CheckMode = iota
CheckInclude
CheckExclude
)

func (c CheckMode) String() string {
switch c {
case CheckAll:
return "CheckAll"
case CheckInclude:
return "CheckInclude"
case CheckExclude:
return "CheckExclude"
default:
return "CheckUnknown"
}
}

type File struct {
Name string
Content string
CommentPrefix []string
Pragmas map[int]*Pragma

CheckMode CheckMode
IssueCodes []string // issue codes to include / exclude based on the CheckMode.
}

func NewFile(content string, commentPrefix []string) *File {
func NewFile(name, content string, commentPrefix []string) *File {
file := &File{
Name: strings.TrimSuffix(name, filepath.Ext(name)),
Content: content,
CommentPrefix: commentPrefix,
Pragmas: make(map[int]*Pragma),
CheckMode: CheckAll,
IssueCodes: nil,
}
file.extractPragmas()
return file
Expand All @@ -45,6 +74,33 @@ func readLine(reader *bufio.Reader) (string, error) {
return lineBuf.String(), nil
}

// readCheckMode takes a comment as an input and checks if it is a check/ignore
// pragma. It should only be called for the first line in the file.
func (f *File) readCheckMode(comment string) {
comment = strings.TrimSpace(comment)

isInclude := strings.HasPrefix(comment, "scatr-check:")
isIgnore := strings.HasPrefix(comment, "scatr-ignore:")

switch {
case isInclude:
comment = strings.TrimPrefix(comment, "scatr-check:")
f.CheckMode = CheckInclude

case isIgnore:
comment = strings.TrimPrefix(comment, "scatr-ignore:")
f.CheckMode = CheckExclude

default:
return
}

for _, issueCode := range strings.Split(comment, ",") {
code := strings.TrimSpace(issueCode)
f.IssueCodes = append(f.IssueCodes, code)
}
}

func (f *File) extractPragmas() {
reader := bufio.NewReader(strings.NewReader(f.Content))

Expand Down Expand Up @@ -73,6 +129,10 @@ func (f *File) extractPragmas() {
continue
}

if lineNum == 1 {
f.readCheckMode(split[1])
}

pragma = ParsePragma(split[1])
if pragma != nil {
if strings.HasPrefix(line, prefix) {
Expand Down
93 changes: 92 additions & 1 deletion pragma/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,101 @@ issueCaseRaised(010);`,
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := NewFile(tt.args.content, tt.args.commentPrefix); !reflect.DeepEqual(got.Pragmas, tt.want) {
if got := NewFile("", tt.args.content, tt.args.commentPrefix); !reflect.DeepEqual(got.Pragmas, tt.want) {
t.Errorf("NewFile() = %v, want %v, diff %v", got.Pragmas, tt.want,
cmp.Diff(tt.want, got.Pragmas))
}
})
}
}

func TestNewFile__CheckMode(t *testing.T) {
type args struct {
name string
content string
commentPrefix []string
}
type want struct {
checkMode CheckMode
issueCodes []string
}
tests := []struct {
name string
args args
want want
}{
{
name: "check all issues - go",
args: args{
name: "file.go",
content: `package main
func main() {}`,
commentPrefix: []string{"//"},
},
want: want{
checkMode: CheckAll,
issueCodes: nil,
},
},
{
name: "check all issues - python",
args: args{
name: "file.py",
content: `print("Hello World")`,
commentPrefix: []string{"#"},
},
want: want{
checkMode: CheckAll,
},
},
{
name: "include issues - pragma",
args: args{
name: "file.py",
content: `# scatr-check: PY-W1000, PY-S1024,PY-1234, issue-code`,
commentPrefix: []string{"#"},
},
want: want{
checkMode: CheckInclude,
issueCodes: []string{"PY-W1000", "PY-S1024", "PY-1234", "issue-code"},
},
},
{
name: "exclude issues",
args: args{
name: "file.py",
content: `# scatr-ignore: PY-W1000, PY-S1024,PY-1234`,
commentPrefix: []string{"#"},
},
want: want{
checkMode: CheckExclude,
issueCodes: []string{"PY-W1000", "PY-S1024", "PY-1234"},
},
},
{
name: "invalid pragma",
args: args{
name: "file.py",
content: `# pragma needs to be on the first line
# scatr-ignore: PY-W1000, PY-S1024,PY-1234`,
commentPrefix: []string{"#"},
},
want: want{
checkMode: CheckAll,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := NewFile(tt.args.name, tt.args.content, tt.args.commentPrefix)

if !reflect.DeepEqual(got.CheckMode, tt.want.checkMode) {
t.Errorf("NewFile().CheckMode = %v, want %v", got.CheckMode, tt.want.checkMode)
}
if !reflect.DeepEqual(got.IssueCodes, tt.want.issueCodes) {
t.Errorf("NewFile().IssueCodes = %v, want %v", got.IssueCodes, tt.want.issueCodes)
}
})
}
}
100 changes: 82 additions & 18 deletions runner/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,27 @@ func newIssuesForFile() *issuesForFile {
}
}

// matchFileNameIssueCodes matches if the file name matches an issue code from
// the analysis result. In case it does, and the check mode is pragma.CheckAll,
// it changes the check mode to pragma.CheckInclude and adds the matched issue
// code to the file's issue list.
func matchFileNameIssueCodes(files map[string]*pragma.File, analysisResult *Result) {
analysisIssueCodes := make(map[string]struct{})
for _, iss := range analysisResult.Issues {
analysisIssueCodes[iss.Code] = struct{}{}
}

for _, file := range files {
_, exists := analysisIssueCodes[file.Name]
if !exists {
continue
}

file.IssueCodes = []string{file.Name}
file.CheckMode = pragma.CheckInclude
}
}

func diffChecksResult(
files map[string]*pragma.File,
excludedDirs []string,
Expand All @@ -35,6 +56,8 @@ func diffChecksResult(
result := make(checksDiff)
passed := true

matchFileNameIssueCodes(files, analysisResult)

for _, iss := range analysisResult.Issues {
if isExcluded(iss.Position.fileNormalized, excludedDirs) {
continue
Expand All @@ -52,24 +75,31 @@ func diffChecksResult(
continue
}

issues.Unexpected = append(issues.Unexpected, iss)
passed = false
if shouldReport(f, iss.Code) {
issues.Unexpected = append(issues.Unexpected, iss)
passed = false
}
continue
}

p, ok := f.Pragmas[iss.Position.Start.Line]
if !ok {
issues.Unexpected = append(issues.Unexpected, iss)
passed = false
if shouldReport(f, iss.Code) {
issues.Unexpected = append(issues.Unexpected, iss)
passed = false
}
continue
}

pragmaIssues, ok := p.Issues[iss.Code]
if !ok {
// issue code mismatch
// if shouldReport(f, iss.Code) {
// fmt.Println("Unexpected c:", iss.Code)
issues.Unexpected = append(issues.Unexpected, iss)
p.Hit[iss.Code] = true
passed = false
// }
p.Hit[iss.Code] = true
continue
}

Expand All @@ -90,8 +120,10 @@ func diffChecksResult(
}

if issueFromPragma == nil {
issues.Unexpected = append(issues.Unexpected, iss)
passed = false
if shouldReport(f, iss.Code) {
issues.Unexpected = append(issues.Unexpected, iss)
passed = false
}
p.Hit[iss.Code] = true
continue
}
Expand All @@ -116,23 +148,25 @@ func diffChecksResult(
for _, issue := range pragmaIssues {
if !issue.Hit {
p.Hit[code] = true
issues.NotRaised = append(issues.NotRaised, &Issue{
Code: code,
Title: issue.Message,
Position: IssuePosition{
Start: Location{
Line: line,
Column: issue.Column,
if shouldReport(file, code) {
issues.NotRaised = append(issues.NotRaised, &Issue{
Code: code,
Title: issue.Message,
Position: IssuePosition{
Start: Location{
Line: line,
Column: issue.Column,
},
},
},
})
passed = false
})
passed = false
}
}
}
}

for code, hit := range p.Hit {
if !hit {
if !hit && shouldReport(file, code) {
issues.NotRaised = append(issues.NotRaised, &Issue{
Code: code,
Title: "",
Expand All @@ -149,6 +183,36 @@ func diffChecksResult(
return result, passed
}

func shouldReport(file *pragma.File, issueCode string) bool {
if file == nil ||
file.CheckMode == pragma.CheckAll ||
file.IssueCodes == nil {
return true
}

switch file.CheckMode {
case pragma.CheckInclude:
for _, code := range file.IssueCodes {
if code == issueCode {
return true
}
}

return false

case pragma.CheckExclude:
for _, code := range file.IssueCodes {
if code == issueCode {
return false
}
}

return true
}

return true
}

type autofixDiff map[string]gotextdiff.Unified

func diffAutofixResult(
Expand Down
4 changes: 3 additions & 1 deletion runner/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,5 +109,7 @@ func getPragmasForFile(path string, commentPrefix []string) (*pragma.File, error
if err != nil {
return nil, err
}
return pragma.NewFile(string(b), commentPrefix), nil

_, name := filepath.Split(path)
return pragma.NewFile(name, string(b), commentPrefix), nil
}
1 change: 1 addition & 0 deletions runner/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestTestChecks(t *testing.T) {
"go", "go_failing", "go_failing_misc",
"go_multiple_pragmas", "go_failing_multiple_files", "go_included_files",
"go_code_path", "go_code_path_included_files", "go_excluded_dirs",
"go_issue_codes",
"py", "py_failing",
}

Expand Down
Loading

0 comments on commit b234906

Please sign in to comment.