Skip to content

Commit

Permalink
Simplify internal comment representation.
Browse files Browse the repository at this point in the history
  • Loading branch information
Denis Krivak committed Jan 9, 2021
1 parent 661f789 commit 3e2bc6f
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 64 deletions.
49 changes: 17 additions & 32 deletions checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,16 @@ var (

// checkComments checks every comment accordings to the rules from
// `settings` argument.
func checkComments(fset *token.FileSet, comments []comment, settings Settings) []Issue {
func checkComments(comments []comment, settings Settings) []Issue {
var issues []Issue // nolint: prealloc
for _, c := range comments {
if c.ast == nil || len(c.ast.List) == 0 {
continue
}

if settings.Period {
if iss := checkCommentForPeriod(fset, c); iss != nil {
if iss := checkCommentForPeriod(c); iss != nil {
issues = append(issues, *iss)
}
}

if settings.Capital {
if iss := checkCommentForCapital(fset, c); len(iss) > 0 {
if iss := checkCommentForCapital(c); len(iss) > 0 {
issues = append(issues, iss...)
}
}
Expand All @@ -54,29 +49,24 @@ func checkComments(fset *token.FileSet, comments []comment, settings Settings) [

// checkCommentForPeriod checks that the last sentense of the comment ends
// in a period.
func checkCommentForPeriod(fset *token.FileSet, c comment) *Issue {
// Save global line number and indentation
start := fset.Position(c.ast.List[0].Slash)

text := getText(c.ast)

pos, ok := checkPeriod(text)
func checkCommentForPeriod(c comment) *Issue {
pos, ok := checkPeriod(c.text)
if ok {
return nil
}

// Shift position by the length of comment's special symbols: /* or //
isBlock := strings.HasPrefix(c.ast.List[0].Text, "/*")
isBlock := strings.HasPrefix(c.lines[0], "/*")
if (isBlock && pos.line == 1) || !isBlock {
pos.column += 2
}

iss := Issue{
Pos: token.Position{
Filename: start.Filename,
Offset: start.Offset,
Line: pos.line + start.Line - 1,
Column: pos.column + start.Column - 1,
Filename: c.start.Filename,
Offset: c.start.Offset,
Line: pos.line + c.start.Line - 1,
Column: pos.column + c.start.Column - 1,
},
Message: noPeriodMessage,
}
Expand All @@ -98,31 +88,26 @@ func checkCommentForPeriod(fset *token.FileSet, c comment) *Issue {
// checkCommentForCapital checks that the each sentense of the comment starts with
// a capital letter.
// nolint: unparam
func checkCommentForCapital(fset *token.FileSet, c comment) []Issue {
// Save global line number and indent
start := fset.Position(c.ast.List[0].Slash)

text := getText(c.ast)

pp := checkCapital(text, c.decl)
func checkCommentForCapital(c comment) []Issue {
pp := checkCapital(c.text, c.decl)
if len(pp) == 0 {
return nil
}

issues := make([]Issue, len(pp))
for i, pos := range pp {
// Shift position by the length of comment's special symbols: /* or //
isBlock := strings.HasPrefix(c.ast.List[0].Text, "/*")
isBlock := strings.HasPrefix(c.lines[0], "/*")
if (isBlock && pos.line == 1) || !isBlock {
pos.column += 2
}

iss := Issue{
Pos: token.Position{
Filename: start.Filename,
Offset: start.Offset,
Line: pos.line + start.Line - 1,
Column: pos.column + start.Column - 1,
Filename: c.start.Filename,
Offset: c.start.Offset,
Line: pos.line + c.start.Line - 1,
Column: pos.column + c.start.Column - 1,
},
Message: noCapitalMessage,
}
Expand Down
18 changes: 1 addition & 17 deletions checks_test.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,6 @@
package godot

import (
"go/ast"
"testing"
)

func TestCheckComments(t *testing.T) {
// Check only case with empty input, other cases are checked in TestRun

comments := []comment{
{ast: nil},
{ast: &ast.CommentGroup{List: nil}},
}
issues := checkComments(nil, comments, Settings{})
if len(issues) > 0 {
t.Fatalf("Unexpected issues: %d", len(issues))
}
}
import "testing"

func TestCheckPeriod(t *testing.T) {
testCases := []struct {
Expand Down
25 changes: 19 additions & 6 deletions getters.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ func getBlockComments(file *ast.File, fset *token.FileSet, lines []string) []com
continue
}
for _, c := range file.Comments {
if c == nil || len(c.List) == 0 {
continue
}
// Skip comments outside this block
if d.Lparen > c.Pos() || c.Pos() > d.Rparen {
continue
Expand All @@ -87,8 +90,9 @@ func getBlockComments(file *ast.File, fset *token.FileSet, lines []string) []com
firstLine := fset.Position(c.Pos()).Line
lastLine := fset.Position(c.End()).Line
comments = append(comments, comment{
ast: c,
lines: lines[firstLine-1 : lastLine],
text: getText(c),
start: fset.Position(c.List[0].Slash),
})
}
}
Expand All @@ -99,14 +103,18 @@ func getBlockComments(file *ast.File, fset *token.FileSet, lines []string) []com
func getTopLevelComments(file *ast.File, fset *token.FileSet, lines []string) []comment {
var comments []comment // nolint: prealloc
for _, c := range file.Comments {
if c == nil || len(c.List) == 0 {
continue
}
if fset.Position(c.Pos()).Column != 1 {
continue
}
firstLine := fset.Position(c.Pos()).Line
lastLine := fset.Position(c.End()).Line
comments = append(comments, comment{
ast: c,
lines: lines[firstLine-1 : lastLine],
text: getText(c),
start: fset.Position(c.List[0].Slash),
})
}
return comments
Expand All @@ -124,15 +132,16 @@ func getDeclarationComments(file *ast.File, fset *token.FileSet, lines []string)
cg = d.Doc
}

if cg == nil {
if cg == nil || len(cg.List) == 0 {
continue
}

firstLine := fset.Position(cg.Pos()).Line
lastLine := fset.Position(cg.End()).Line
comments = append(comments, comment{
ast: cg,
lines: lines[firstLine-1 : lastLine],
text: getText(cg),
start: fset.Position(cg.List[0].Slash),
})
}
return comments
Expand All @@ -142,11 +151,15 @@ func getDeclarationComments(file *ast.File, fset *token.FileSet, lines []string)
func getAllComments(file *ast.File, fset *token.FileSet, lines []string) []comment {
var comments []comment //nolint: prealloc
for _, c := range file.Comments {
if c == nil || len(c.List) == 0 {
continue
}
firstLine := fset.Position(c.Pos()).Line
lastLine := fset.Position(c.End()).Line
comments = append(comments, comment{
ast: c,
lines: lines[firstLine-1 : lastLine],
start: fset.Position(c.List[0].Slash),
text: getText(c),
})
}
return comments
Expand Down Expand Up @@ -203,7 +216,7 @@ func readFile(file *ast.File, fset *token.FileSet) ([]string, error) {
func setDecl(comments, decl []comment) {
for _, d := range decl {
for i, c := range comments {
if d.ast.Pos() == c.ast.Pos() {
if d.start == c.start {
comments[i].decl = true
break
}
Expand Down
14 changes: 11 additions & 3 deletions getters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,11 @@ func TestGetComments(t *testing.T) {
}
var expected int
for _, c := range comments {
if strings.Contains(c.ast.Text(), "[NONE]") {
if linesContain(c.lines, "[NONE]") {
continue
}
text := c.ast.Text()
for _, s := range tt.contains {
if strings.Contains(text, s) {
if strings.Contains(c.text, s) {
expected++
break
}
Expand Down Expand Up @@ -192,3 +191,12 @@ func TestGetText(t *testing.T) {
})
}
}

func linesContain(lines []string, s string) bool {
for _, ln := range lines {
if strings.Contains(ln, s) {
return true
}
}
return false
}
13 changes: 7 additions & 6 deletions godot.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ type position struct {
column int
}

// comment is an internal representation of AST comment entity with rendered
// lines attached. The latter is used for creating a full replacement for
// comment is an internal representation of AST comment entity with additional
// data attached. The latter is used for creating a full replacement for
// the line with issues.
type comment struct {
ast *ast.CommentGroup
lines []string
decl bool
lines []string // unmodified lines from file
text string // concatenated `lines` with special parts excluded
start token.Position // position of the first symbol in comment
decl bool // whether comment is a special one (should not be checked)
}

// Run runs this linter on the provided code.
Expand All @@ -46,7 +47,7 @@ func Run(file *ast.File, fset *token.FileSet, settings Settings) ([]Issue, error
return nil, fmt.Errorf("get comments: %v", err)
}

issues := checkComments(fset, comments, settings)
issues := checkComments(comments, settings)
sortIssues(issues)

return issues, nil
Expand Down

0 comments on commit 3e2bc6f

Please sign in to comment.