Skip to content

Commit

Permalink
exported err variables should be of type error
Browse files Browse the repository at this point in the history
It's not good practice to match errors based on their `.String()`
value. If the package's exporter `ErrXxx..` are of type `string`,
it encourages users to do just that.

Make the `ErrXxx` be of type `error`.
  • Loading branch information
aybabtme committed Feb 20, 2017
1 parent 027a317 commit 92b0da7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
14 changes: 8 additions & 6 deletions license.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ const (
LicenseCDDL10 = "CDDL-1.0"
LicenseEPL10 = "EPL-1.0"
LicenseUnlicense = "Unlicense"
)

var (
// Various errors
ErrNoLicenseFile = "license: unable to find any license file"
ErrUnrecognizedLicense = "license: could not guess license type"
ErrMultipleLicenses = "license: multiple license files found"
ErrNoLicenseFile = errors.New("license: unable to find any license file")
ErrUnrecognizedLicense = errors.New("license: could not guess license type")
ErrMultipleLicenses = errors.New("license: multiple license files found")
)

// A set of reasonable license file names to use when guessing where the
Expand Down Expand Up @@ -210,7 +212,7 @@ func (l *License) GuessType() error {
l.Type = LicenseUnlicense

default:
return errors.New(ErrUnrecognizedLicense)
return ErrUnrecognizedLicense
}

return nil
Expand Down Expand Up @@ -257,10 +259,10 @@ func getLicenseFile(licenses []string, files []string) (string, error) {

switch len(matches) {
case 0:
return "", errors.New(ErrNoLicenseFile)
return "", ErrNoLicenseFile
case 1:
return matches[0], nil
default:
return "", errors.New(ErrMultipleLicenses)
return "", ErrMultipleLicenses
}
}
7 changes: 3 additions & 4 deletions license_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package license

import (
"errors"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -213,9 +212,9 @@ func TestGetLicenseFile(t *testing.T) {
want string
err error
}{
{[]string{".", "junk", "COPYING"}, "COPYING", nil}, // 1 match
{[]string{"junk", "copy"}, "", errors.New(ErrNoLicenseFile)}, // 0 match
{[]string{"COPYING", "junk", "Copying.txt"}, "", errors.New(ErrMultipleLicenses)}, // 2 match
{[]string{".", "junk", "COPYING"}, "COPYING", nil}, // 1 match
{[]string{"junk", "copy"}, "", ErrNoLicenseFile}, // 0 match
{[]string{"COPYING", "junk", "Copying.txt"}, "", ErrMultipleLicenses}, // 2 match
}

for pos, tt := range tests {
Expand Down

0 comments on commit 92b0da7

Please sign in to comment.