Permalink
Browse files

Flag error strings that end with a newline

Signed-off-by: Joe Tsai <joetsai@google.com>
1 parent 3390df4 commit 206c0f020eba0f7fbcfbc467a5eb808037df2ed6 @dominikh dominikh committed with dsnet Nov 14, 2016
Showing with 30 additions and 32 deletions.
  1. +23 −26 lint.go
  2. +7 −6 testdata/errors.go
View
@@ -1155,21 +1155,30 @@ func (f *file) lintErrors() {
}
}
-func lintCapAndPunct(s string) (isCap, isPunct bool) {
+func lintErrorString(s string) (isClean bool, conf float64) {
+ const basicConfidence = 0.8
+ const capConfidence = basicConfidence - 0.2
first, firstN := utf8.DecodeRuneInString(s)
last, _ := utf8.DecodeLastRuneInString(s)
- isPunct = last == '.' || last == ':' || last == '!'
- isCap = unicode.IsUpper(first)
- if isCap && len(s) > firstN {
- // Don't flag strings starting with something that looks like an initialism.
- if second, _ := utf8.DecodeRuneInString(s[firstN:]); unicode.IsUpper(second) {
- isCap = false
+ if last == '.' || last == ':' || last == '!' || last == '\n' {
+ return false, basicConfidence
+ }
+ if unicode.IsUpper(first) {
+ // People use proper nouns and exported Go identifiers in error strings,
+ // so decrease the confidence of warnings for capitalization.
+ if len(s) <= firstN {
+ return false, capConfidence
+ }
+ // Flag strings starting with something that doesn't look like an initialism.
+ if second, _ := utf8.DecodeRuneInString(s[firstN:]); !unicode.IsUpper(second) {
+ return false, capConfidence
}
}
- return
+ return true, 0
}
-// lintErrorStrings examines error strings. It complains if they are capitalized or end in punctuation.
+// lintErrorStrings examines error strings.
+// It complains if they are capitalized or end in punctuation or a newline.
func (f *file) lintErrorStrings() {
f.walk(func(node ast.Node) bool {
ce, ok := node.(*ast.CallExpr)
@@ -1190,25 +1199,13 @@ func (f *file) lintErrorStrings() {
if s == "" {
return true
}
- isCap, isPunct := lintCapAndPunct(s)
- var msg string
- switch {
- case isCap && isPunct:
- msg = "error strings should not be capitalized and should not end with punctuation"
- case isCap:
- msg = "error strings should not be capitalized"
- case isPunct:
- msg = "error strings should not end with punctuation"
- default:
+ clean, conf := lintErrorString(s)
+ if clean {
return true
}
- // People use proper nouns and exported Go identifiers in error strings,
- // so decrease the confidence of warnings for capitalization.
- conf := 0.8
- if isCap {
- conf = 0.6
- }
- f.errorf(str, conf, link(styleGuideBase+"#error-strings"), category("errors"), msg)
+
+ f.errorf(str, conf, link(styleGuideBase+"#error-strings"), category("errors"),
+ "error strings should not be capitalized or end with punctuation or a newline")
return true
})
}
View
@@ -27,10 +27,11 @@ func f() {
// Check for the error strings themselves.
func g(x int) error {
- if x < 1 {
- return fmt.Errorf("This %d is too low", x) // MATCH /error strings.*not be capitalized/
- } else if x == 0 {
- return fmt.Errorf("XML time") // ok
- }
- return errors.New(`too much stuff.`) // MATCH /error strings.*not end with punctuation/
+ var err error
+ err = fmt.Errorf("This %d is too low", x) // MATCH /error strings.*be capitalized/
+ err = fmt.Errorf("XML time") // ok
+ err = fmt.Errorf("newlines are fun\n") // MATCH /error strings.*end with punctuation/
+ err = fmt.Errorf("Newlines are really fun\n") // MATCH /error strings.+not be capitalized/
+ err = errors.New(`too much stuff.`) // MATCH /error strings.*end with punctuation/
+ return err
}

3 comments on commit 206c0f0

@glasser

Note that as a side effect of this commit, errors that both are capitalized and end in punctuation are reported at confidence level 0.8 rather than 0.6 (ie, will be shown by default). This is a reasonable change but I think it's worth mentioning here.

@dsnet
Member
dsnet commented on 206c0f0 Nov 15, 2016

Thanks for the mention. I think the new behavior is better.

@glasser

Yup. Just noticed it because I was surprised to see a lint failure show up in a formerly lint-free file!

Please sign in to comment.