Skip to content

Commit

Permalink
adjust format
Browse files Browse the repository at this point in the history
  • Loading branch information
notJoon committed Oct 28, 2024
1 parent 38d3b26 commit 5ebdf06
Show file tree
Hide file tree
Showing 18 changed files with 83 additions and 65 deletions.
29 changes: 21 additions & 8 deletions formatter/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ var (
warningStyle = color.New(color.FgHiYellow, color.Bold)
ruleStyle = color.New(color.FgYellow, color.Bold)
fileStyle = color.New(color.FgCyan, color.Bold)
lineStyle = color.New(color.FgBlue, color.Bold)
lineStyle = color.New(color.FgHiBlue, color.Bold)
messageStyle = color.New(color.FgRed, color.Bold)
suggestionStyle = color.New(color.FgGreen, color.Bold)
noStyle = color.New(color.FgWhite)
)

// issueFormatter is the interface that wraps the Format method.
Expand Down Expand Up @@ -136,7 +137,8 @@ func (b *issueFormatterBuilder) AddCodeSnippet() *issueFormatterBuilder {
line = strings.TrimPrefix(line, b.commonIndent)
lineNum := fmt.Sprintf("%*d", b.maxLineNumWidth, i)

b.writeStyledLine(lineStyle, "%s | %s\n", lineNum, line)
b.writeStyledLine(lineStyle, "%s | ", lineNum)
b.writeStyledLine(noStyle, "%s\n", line)
}

return b
Expand All @@ -163,10 +165,15 @@ func (b *issueFormatterBuilder) AddUnderlineAndMessage() *issueFormatterBuilder
underlineLength := underlineEnd - underlineStart + 1

b.result.WriteString(strings.Repeat(" ", underlineStart))
b.writeStyledLine(messageStyle, "%s\n", strings.Repeat("~", underlineLength))
b.writeStyledLine(messageStyle, "%s\n", strings.Repeat("^", underlineLength))
b.writeStyledLine(lineStyle, "%s|\n", b.padding)

b.writeStyledLine(lineStyle, "%s= ", b.padding)
b.writeStyledLine(messageStyle, "%s\n\n", b.issue.Message)
b.writeStyledLine(messageStyle, "%s\n", b.issue.Message)

if b.issue.Note == "" {
b.result.WriteString("\n")
}

return b
}
Expand All @@ -182,13 +189,14 @@ func (b *issueFormatterBuilder) AddSuggestion() *issueFormatterBuilder {
return b
}

b.writeStyledLine(suggestionStyle, "Suggestion:\n")
b.writeStyledLine(suggestionStyle, "suggestion:\n")
b.writeStyledLine(lineStyle, "%s|\n", b.padding)

suggestionLines := strings.Split(b.issue.Suggestion, "\n")
for i, line := range suggestionLines {
lineNum := fmt.Sprintf("%*d", b.maxLineNumWidth, b.issue.Start.Line+i)
b.writeStyledLine(lineStyle, "%s | %s\n", lineNum, line)
b.writeStyledLine(lineStyle, "%s | ", lineNum)
b.writeStyledLine(noStyle, "%s\n", line)
}

b.writeStyledLine(lineStyle, "%s|\n\n", b.padding)
Expand All @@ -201,8 +209,13 @@ func (b *issueFormatterBuilder) AddNote() *issueFormatterBuilder {
return b
}

b.result.WriteString(suggestionStyle.Sprint("Note: "))
b.writeStyledLine(lineStyle, "%s\n\n", b.issue.Note)
b.writeStyledLine(lineStyle, "%s= ", b.padding)
b.result.WriteString(noStyle.Sprint("note: "))

b.writeStyledLine(noStyle, "%s\n", b.issue.Note)
if b.issue.Suggestion == "" {
b.result.WriteString("\n")
}

return b
}
Expand Down
2 changes: 1 addition & 1 deletion formatter/cyclomatic_complexity.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ func (f *CyclomaticComplexityFormatter) Format(issue tt.Issue, snippet *internal
AddHeader().
AddCodeSnippet().
AddComplexityInfo().
AddSuggestion().
AddNote().
AddSuggestion().
Build()
}

Expand Down
30 changes: 18 additions & 12 deletions formatter/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,16 @@ func TestFormatIssuesWithArrows(t *testing.T) {
--> test.go:4:5
|
4 | x := 1
| ~~
| ^^
|
= x declared but not used
error: empty-if
--> test.go:5:5
|
5 | if true {}
| ~~~~~~~~~
| ^^^^^^^^^
|
= empty branch
`
Expand All @@ -75,14 +77,16 @@ error: empty-if
--> test.go:4:5
|
4 | x := 1
| ~~
| ^^
|
= x declared but not used
error: empty-if
--> test.go:5:5
|
5 | if true {}
| ~~~~~~~~~
| ^^^^^^^^^
|
= empty branch
`
Expand Down Expand Up @@ -137,21 +141,24 @@ func TestFormatIssuesWithArrows_MultipleDigitsLineNumbers(t *testing.T) {
--> test.go:4:5
|
4 | x := 1 // unused variable
| ~~
| ^^
|
= x declared but not used
error: empty-if
--> test.go:5:5
|
5 | if true {} // empty if statement
| ~~~~~~~~~
| ^^^^^^^^^
|
= empty branch
error: example
--> test.go:10:5
|
10 | println("end")
| ~~~~~~~~
| ^^^^^^^^
|
= example issue
`
Expand Down Expand Up @@ -190,16 +197,15 @@ func TestUnnecessaryTypeConversionFormatter(t *testing.T) {
--> test.go:5:10
|
5 | result := int(myInt)
| ~~~~~~~~~~~
| ^^^^^^^^^^^
|
= unnecessary type conversion
Suggestion:
= note: Unnecessary type conversions can make the code less readable and may slightly impact performance. They are safe to remove when the expression already has the desired type.
suggestion:
|
5 | Remove the type conversion. Change ` + "`int(myInt)`" + ` to just ` + "`myInt`" + `.
|
Note: Unnecessary type conversions can make the code less readable and may slightly impact performance. They are safe to remove when the expression already has the desired type.
`

result := formatter.Format(issue, snippet)
Expand Down
2 changes: 1 addition & 1 deletion formatter/general.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (f *GeneralIssueFormatter) Format(
AddHeader().
AddCodeSnippet().
AddUnderlineAndMessage().
AddSuggestion().
AddNote().
AddSuggestion().
Build()
}
2 changes: 1 addition & 1 deletion internal/lints/const_error_decl.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func DetectConstErrorDeclaration(
Filename: filename,
Start: fset.Position(genDecl.Pos()),
End: fset.Position(genDecl.End()),
Message: "Avoid declaring constant errors",
Message: "avoid declaring constant errors",
Suggestion: suggestion,
Confidence: 1.0,
Severity: severity,
Expand Down
4 changes: 2 additions & 2 deletions internal/lints/cyclomatic_complexity.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func DetectHighCyclomaticComplexity(filename string, threshold int, severity tt.
Start: fset.Position(funcNode.Pos()),
End: fset.Position(funcNode.End()),
Message: fmt.Sprintf("function %s has a cyclomatic complexity of %d (threshold %d)", stat.FuncName, stat.Complexity, threshold),
Suggestion: "Consider refactoring this function to reduce its complexity. You can split it into smaller functions or simplify the logic.\n",
Note: "High cyclomatic complexity can make the code harder to understand, test, and maintain. Aim for a complexity score of 10 or less for most functions.\n",
Suggestion: "consider refactoring this function to reduce its complexity. you can split it into smaller functions or simplify the logic.\n",
Note: "high cyclomatic complexity can make the code harder to understand, test, and maintain. aim for a complexity score of 10 or less for most functions.\n",
Severity: severity,
}
issues = append(issues, issue)
Expand Down
34 changes: 17 additions & 17 deletions internal/lints/defers.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ func (dc *DeferChecker) checkDeferPanic(stmt *ast.DeferStmt) {
if call, ok := n.(*ast.CallExpr); ok {
if ident, ok := call.Fun.(*ast.Ident); ok && ident.Name == "panic" {
dc.addIssue("defer-panic", stmt.Pos(), stmt.End(),
"Avoid calling panic inside a defer statement",
"Consider removing the panic call from the defer statement. "+
"If error handling is needed, use a separate error check before the defer.")
"avoid calling panic inside a defer statement",
"consider removing the panic call from the defer statement. "+
"if error handling is needed, use a separate error check before the defer.")
}
}
return true
Expand All @@ -56,8 +56,8 @@ func (dc *DeferChecker) checkDeferNilFunc(stmt *ast.DeferStmt) {
if ident, ok := stmt.Call.Fun.(*ast.Ident); ok {
if ident.Name == "nil" || (ident.Obj != nil && ident.Obj.Kind == ast.Var) {
dc.addIssue("defer-nil-func", stmt.Pos(), stmt.End(),
"Avoid deferring a potentially nil function",
"Deferring a nil function will cause a panic at runtime. Ensure the function is not nil before deferring.")
"avoid deferring a potentially nil function",
"deferring a nil function will cause a panic at runtime. ensure the function is not nil before deferring.")
}
}
}
Expand All @@ -68,8 +68,8 @@ func (dc *DeferChecker) checkReturnInDefer(stmt *ast.DeferStmt) {
ast.Inspect(funcLit.Body, func(n ast.Node) bool {
if _, ok := n.(*ast.ReturnStmt); ok {
dc.addIssue("return-in-defer", n.Pos(), n.End(),
"Avoid using return statement inside a defer function",
"The return statement in a deferred function doesn't affect the returned value of the surrounding function. Consider removing it or refactoring your code.")
"avoid using return statement inside a defer function",
"the return statement in a deferred function doesn't affect the returned value of the surrounding function. consider removing it or refactoring your code.")
return false
}
return true
Expand All @@ -87,23 +87,23 @@ func (dc *DeferChecker) checkDeferInLoop(n ast.Node) {
ast.Inspect(n, func(inner ast.Node) bool {
if defer_, ok := inner.(*ast.DeferStmt); ok {
dc.addIssue("defer-in-loop", defer_.Pos(), defer_.End(),
"Avoid using defer inside a loop",
"Consider moving the defer statement outside the loop to avoid potential performance issues.")
"avoid using defer inside a loop",
"consider moving the defer statement outside the loop to avoid potential performance issues.")
}
return true
})
}
}

func (dc *DeferChecker) addIssue(rule string, start, end token.Pos, message, suggestion string) {
func (dc *DeferChecker) addIssue(rule string, start, end token.Pos, message, note string) {
dc.issues = append(dc.issues, tt.Issue{
Rule: rule,
Filename: dc.filename,
Start: dc.fset.Position(start),
End: dc.fset.Position(end),
Message: message,
Suggestion: suggestion,
Severity: dc.severity,
Rule: rule,
Filename: dc.filename,
Start: dc.fset.Position(start),
End: dc.fset.Position(end),
Message: message,
Note: note,
Severity: dc.severity,
})
}

Expand Down
4 changes: 2 additions & 2 deletions internal/lints/deprecate_func.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ func DetectDeprecatedFunctions(
func createDeprecationMessage(df checker.DeprecatedFunc) string {
msg := "Use of deprecated function"
if df.Alternative != "" {
msg = fmt.Sprintf("%s. Please use %s instead.", msg, df.Alternative)
msg = fmt.Sprintf("%s. please use %s instead.", msg, df.Alternative)
return msg
}
msg = fmt.Sprintf("%s. Please remove it.", msg)
msg = fmt.Sprintf("%s. please remove it.", msg)
return msg
}

Expand Down
2 changes: 1 addition & 1 deletion internal/lints/detect_cycles.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func DetectCycle(filename string, node *ast.File, fset *token.FileSet, severity
Filename: filename,
Start: fset.Position(node.Pos()),
End: fset.Position(node.End()),
Message: "Detected cycle in function call: " + cycle,
Message: "detected cycle in function call: " + cycle,
Severity: severity,
}
issues = append(issues, issue)
Expand Down
2 changes: 1 addition & 1 deletion internal/lints/early_return.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func DetectEarlyReturnOpportunities(filename string, node *ast.File, fset *token
Filename: filename,
Start: fset.Position(ifStmt.Pos()),
End: fset.Position(ifStmt.End()),
Message: "This if-else chain can be simplified using early returns",
Message: "this if-else chain can be simplified using early returns",
Suggestion: suggestion,
Confidence: 0.8,
Severity: severity,
Expand Down
1 change: 0 additions & 1 deletion internal/lints/early_return_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ func example(x int) {
issues, err := DetectEarlyReturnOpportunities(tmpfile, node, fset, types.SeverityError)
require.NoError(t, err)

// assert.Equal(t, tt.expected, len(issues), "Number of detected early return opportunities doesn't match expected")
if len(issues) != tt.expected {
for _, issue := range issues {
t.Logf("Issue: %v", issue)
Expand Down
2 changes: 1 addition & 1 deletion internal/lints/format_emit.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func DetectEmitFormat(filename string, node *ast.File, fset *token.FileSet, seve
Filename: filename,
Start: fset.Position(call.Pos()),
End: fset.Position(call.End()),
Message: "Consider formatting std.Emit call for better readability",
Message: "consider formatting std.Emit call for better readability",
Suggestion: formatEmitCall(call),
Confidence: 1.0,
Severity: severity,
Expand Down
6 changes: 3 additions & 3 deletions internal/lints/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func main() {
assert.Equal(t, tt.expected, len(issues), "Number of issues does not match expected")

for _, issue := range issues {
assert.Contains(t, issue.Message, "Potential unnecessary allocation inside loop")
assert.Contains(t, issue.Message, "potential unnecessary allocation inside loop")
}
})
}
Expand Down Expand Up @@ -383,7 +383,7 @@ func TestDetectEmitFormat(t *testing.T) {

if len(issues) > 0 {
assert.Equal(t, "emit-format", issues[0].Rule)
assert.Contains(t, issues[0].Message, "Consider formatting std.Emit call for better readability")
assert.Contains(t, issues[0].Message, "consider formatting std.Emit call for better readability")
}
})
}
Expand Down Expand Up @@ -721,7 +721,7 @@ var err = errors.New("error")

for _, issue := range issues {
assert.Equal(t, "const-error-declaration", issue.Rule)
assert.Contains(t, issue.Message, "Avoid declaring constant errors")
assert.Contains(t, issue.Message, "avoid declaring constant errors")
assert.Contains(t, issue.Suggestion, "var")
}
})
Expand Down
2 changes: 1 addition & 1 deletion internal/lints/loop_allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func DetectLoopAllocation(filename string, node *ast.File, fset *token.FileSet,
if isAllocationFunction(innerNode) {
issues = append(issues, tt.Issue{
Rule: "loop-allocation",
Message: "Potential unnecessary allocation inside loop",
Message: "potential unnecessary allocation inside loop",
Start: fset.Position(innerNode.Pos()),
End: fset.Position(innerNode.End()),
Severity: severity,
Expand Down
4 changes: 2 additions & 2 deletions internal/lints/missing_package_mod.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func DetectMissingModPackage(filename string, node *ast.File, fset *token.FileSe
Filename: modFile,
Start: token.Position{Filename: modFile},
End: token.Position{Filename: modFile},
Message: fmt.Sprintf("Packages %s are declared in gno.mod file but not imported.\nRun `gno mod tidy`", strings.Join(unusedPackages, ", ")),
Message: fmt.Sprintf("packages %s are declared in gno.mod file but not imported.\nrun `gno mod tidy`", strings.Join(unusedPackages, ", ")),
Severity: severity,
}
issues = append(issues, issue)
Expand All @@ -55,7 +55,7 @@ func DetectMissingModPackage(filename string, node *ast.File, fset *token.FileSe
Filename: modFile,
Start: token.Position{Filename: modFile},
End: token.Position{Filename: modFile},
Message: fmt.Sprintf("Package %s is imported but not declared in gno.mod file. Please consider to remove.\nRun `gno mod tidy`", pkg),
Message: fmt.Sprintf("package %s is imported but not declared in gno.mod file. please consider to remove.\nrun `gno mod tidy`", pkg),
Severity: severity,
}
issues = append(issues, issue)
Expand Down
12 changes: 6 additions & 6 deletions internal/lints/simplify_slice_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,18 @@ func DetectUnnecessarySliceLength(filename string, node *ast.File, fset *token.F
if sliceExpr.Low == nil {
suggestion = fmt.Sprintf("%s[:]", arg.Name)
detailedMessage = fmt.Sprintf(
"%s\nIn this case, `%s[:len(%s)]` is equivalent to `%s[:]`. "+
"The full length of the slice is already implied when omitting both start and end indices.",
"%s\nin this case, `%s[:len(%s)]` is equivalent to `%s[:]`. "+
"the full length of the slice is already implied when omitting both start and end indices.",
baseMessage, arg.Name, arg.Name, arg.Name)
} else if basicLit, ok := sliceExpr.Low.(*ast.BasicLit); ok {
suggestion = fmt.Sprintf("%s[%s:]", arg.Name, basicLit.Value)
detailedMessage = fmt.Sprintf("%s\nHere, `%s[%s:len(%s)]` can be simplified to `%s[%s:]`. "+
"When slicing to the end of a slice, using len() is unnecessary.",
detailedMessage = fmt.Sprintf("%s\nhere, `%s[%s:len(%s)]` can be simplified to `%s[%s:]`. "+
"when slicing to the end of a slice, using len() is unnecessary.",
baseMessage, arg.Name, basicLit.Value, arg.Name, arg.Name, basicLit.Value)
} else if lowIdent, ok := sliceExpr.Low.(*ast.Ident); ok {
suggestion = fmt.Sprintf("%s[%s:]", arg.Name, lowIdent.Name)
detailedMessage = fmt.Sprintf("%s\nIn this instance, `%s[%s:len(%s)]` can be written as `%s[%s:]`. "+
"The len() function is redundant when slicing to the end, regardless of the start index.",
detailedMessage = fmt.Sprintf("%s\nin this instance, `%s[%s:len(%s)]` can be written as `%s[%s:]`. "+
"the len() function is redundant when slicing to the end, regardless of the start index.",
baseMessage, arg.Name, lowIdent.Name, arg.Name, arg.Name, lowIdent.Name)
}

Expand Down
Loading

0 comments on commit 5ebdf06

Please sign in to comment.