Skip to content

Commit

Permalink
lint: Fix readability warnings, like 'lll' and 'wsl'
Browse files Browse the repository at this point in the history
Signed-off-by: Stephen Augustus <saugustus@vmware.com>
  • Loading branch information
justaugustus committed Sep 6, 2020
1 parent 91960ac commit e413356
Show file tree
Hide file tree
Showing 14 changed files with 185 additions and 30 deletions.
23 changes: 13 additions & 10 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,19 @@ linters:
- dogsled
- dupl
- errcheck
- gochecknoglobals
- gochecknoinits
- gocognit
- goconst
- gocritic
- gocyclo
- godox
- gofmt
- goimports
- golint
- gomnd
- goprintffuncname
- gosec
- gosimple
- govet
- ineffassign
Expand All @@ -36,6 +41,7 @@ linters:
- nakedret
- prealloc
- rowserrcheck
- scopelint
- staticcheck
- structcheck
- stylecheck
Expand All @@ -45,15 +51,12 @@ linters:
- unused
- varcheck
- whitespace
# - funlen
# - gochecknoglobals
# - gochecknoinits
# - gocognit
# - gomnd
# - gosec
# - lll
# - scopelint
# - wsl

# Nits: These are things one might mention, but not block on during review.
# We can consider disabling these checks, if we determine them to be
# a little too nitty and obtrusive.
- lll
- wsl
linters-settings:
godox:
keywords:
Expand Down Expand Up @@ -106,6 +109,7 @@ linters-settings:
- emptyFallthrough
- emptyStringTest
- hexLiteral
- ifElseChain
- methodExprCall
- regexpMust
- singleCaseSwitch
Expand All @@ -121,7 +125,6 @@ linters-settings:
- valSwap
- wrapperFunc
- yodaStyleExpr
# - ifElseChain

# Opinionated
- builtinShadow
Expand Down
64 changes: 60 additions & 4 deletions dependencies/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,29 +66,37 @@ type RefPath struct {
func (decoded *Dependency) UnmarshalYAML(unmarshal func(interface{}) error) error {
// Use a different type to prevent infinite loop in unmarshalling
type DependencyYAML Dependency

d := (*DependencyYAML)(decoded)

if err := unmarshal(&d); err != nil {
return err
}

// Custom validation for the Dependency type
if d.Name == "" {
return fmt.Errorf("Dependency has no `name`: %v", d)
}

if d.Version == "" {
return fmt.Errorf("Dependency has no `version`: %v", d)
}

// Default scheme to Semver if unset
if d.Scheme == "" {
d.Scheme = Semver
}

// Validate Scheme and return
switch d.Scheme {
case Semver, Alpha, Random:
// All good!
default:
return fmt.Errorf("unknown version scheme: %s", d.Scheme)
}

log.Debugf("Deserialised Dependency %v: %v", d.Name, d)

return nil
}

Expand All @@ -99,10 +107,12 @@ func fromFile(dependencyFilePath string) (*Dependencies, error) {
}

dependencies := &Dependencies{}

err = yaml.Unmarshal(depFile, dependencies)
if err != nil {
return nil, err
}

return dependencies, nil
}

Expand All @@ -111,42 +121,61 @@ func fromFile(dependencyFilePath string) (*Dependencies, error) {
// Will return an error if the dependency cannot be found in the files it has defined, or if the version does not match
func LocalCheck(dependencyFilePath string) error {
base := filepath.Dir(dependencyFilePath)

externalDeps, err := fromFile(dependencyFilePath)
if err != nil {
return err
}

var nonMatchingPaths []string

for _, dep := range externalDeps.Dependencies {
log.Debugf("Examining dependency: %v", dep.Name)

for _, refPath := range dep.RefPaths {
filePath := filepath.Join(base, refPath.Path)

file, err := os.Open(filePath)
if err != nil {
log.Errorf("Error opening %v: %v", filePath, err)
return err
}

log.Debugf("Examining file: %v", filePath)

match := refPath.Match
matcher := regexp.MustCompile(match)
scanner := bufio.NewScanner(file)

var found bool

var lineNumber int
for scanner.Scan() {
lineNumber++

line := scanner.Text()
if matcher.MatchString(line) {
if strings.Contains(line, dep.Version) {
log.Debugf("Line %v matches expected regexp '%v' and version '%v':\n%v", lineNumber, match, dep.Version, line)
log.Debugf(
"Line %v matches expected regexp '%v' and version '%v':\n%v",
lineNumber,
match,
dep.Version,
line,
)

found = true

break
} else {
log.Warnf("Line %v matches expected regexp '%v', but not version '%v':\n%v", lineNumber, match, dep.Version, line)
}
}
}

if !found {
log.Debugf("Finished reading file %v, no match found.", filePath)

nonMatchingPaths = append(nonMatchingPaths, refPath.Path)
}
}
Expand All @@ -157,6 +186,7 @@ func LocalCheck(dependencyFilePath string) error {
return errors.New("Dependencies are not in sync")
}
}

return nil
}

Expand All @@ -170,53 +200,65 @@ func RemoteCheck(dependencyFilePath string) ([]string, error) {
if err != nil {
return nil, err
}

updates := make([]string, 0)

for _, dep := range externalDeps.Dependencies {
log.Debugf("Examining dependency: %v", dep.Name)

if dep.Upstream == nil {
continue
}
upstream := dep.Upstream

upstream := dep.Upstream
latestVersion := Version{dep.Version, dep.Scheme}
currentVersion := Version{dep.Version, dep.Scheme}

var err error

// Cast the flavour from the currently unknown upstream type
flavour := upstreams.UpstreamFlavour(upstream["flavour"])
switch flavour {
case upstreams.DummyFlavour:
var d upstreams.Dummy

decodeErr := mapstructure.Decode(upstream, &d)
if decodeErr != nil {
return nil, decodeErr
}

latestVersion.Version, err = d.LatestVersion()
case upstreams.GithubFlavour:
var gh upstreams.Github

decodeErr := mapstructure.Decode(upstream, &gh)
if decodeErr != nil {
return nil, decodeErr
}

latestVersion.Version, err = gh.LatestVersion()
case upstreams.AMIFlavour:
var ami upstreams.AMI

decodeErr := mapstructure.Decode(upstream, &ami)
if decodeErr != nil {
return nil, decodeErr
}

latestVersion.Version, err = ami.LatestVersion()
case upstreams.HelmFlavour:
var helm upstreams.Helm

decodeErr := mapstructure.Decode(upstream, &helm)
if decodeErr != nil {
return nil, decodeErr
}

latestVersion.Version, err = helm.LatestVersion()
default:
return nil, fmt.Errorf("unknown upstream flavour '%v' for dependency %v", flavour, dep.Name)
}

if err != nil {
return nil, err
}
Expand All @@ -227,10 +269,24 @@ func RemoteCheck(dependencyFilePath string) ([]string, error) {
}

if updateAvailable {
updates = append(updates, fmt.Sprintf("Update available for dependency %v: %v (current: %v)", dep.Name, latestVersion.Version, currentVersion.Version))
updates = append(
updates,
fmt.Sprintf(
"Update available for dependency %v: %v (current: %v)",
dep.Name,
latestVersion.Version,
currentVersion.Version,
),
)
} else {
log.Debugf("No update available for dependency %v: %v (latest: %v)\n", dep.Name, currentVersion.Version, latestVersion.Version)
log.Debugf(
"No update available for dependency %v: %v (latest: %v)\n",
dep.Name,
currentVersion.Version,
latestVersion.Version,
)
}
}

return updates, nil
}
2 changes: 2 additions & 0 deletions dependencies/dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func TestBrokenFile(t *testing.T) {
if err == nil {
t.Errorf("Did not return an error on trying to open a file that doesn't exist")
}

err = LocalCheck("../testdata/Dockerfile")
if err == nil {
t.Errorf("Did not return an error on trying to open a non-yaml file")
Expand Down Expand Up @@ -107,6 +108,7 @@ func TestDeserialising(t *testing.T) {

for _, valid := range validYamls {
var d Dependency

err := yaml.Unmarshal([]byte(valid), &d)
if err != nil {
t.Errorf("Failed to deserialise valid yaml:\n%s", valid)
Expand Down
10 changes: 8 additions & 2 deletions dependencies/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ const (
Random VersionScheme = "random"
)

// VersionSensitivity informs us on how to compare whether a version is more recent than another, for example to only notify on new major versions
// VersionSensitivity informs us on how to compare whether a version is more
// recent than another, for example to only notify on new major versions
// Only applicable to Semver versioning
type VersionSensitivity string

Expand All @@ -60,27 +61,32 @@ func (a Version) MoreRecentThan(b Version) (bool, error) {
return a.MoreSensitivelyRecentThan(b, Patch)
}

// MoreSensitivelyRecentThan checks whether a given version is more recent than another one, accepting a VersionSensitivity argument
// MoreSensitivelyRecentThan checks whether a given version is more recent than
// another one, accepting a VersionSensitivity argument
//
// If the VersionScheme is "random", then it will return true if a != b.
func (a Version) MoreSensitivelyRecentThan(b Version, sensitivity VersionSensitivity) (bool, error) {
// Default to a Patch-level sensitivity
if sensitivity == "" {
sensitivity = Patch
}

if a.Scheme != b.Scheme {
return false, fmt.Errorf("trying to compare incompatible 'Version' schemes: %s and %s", a.Scheme, b.Scheme)
}

switch a.Scheme {
case Semver:
aSemver, err := semver.Parse(a.Version)
if err != nil {
return false, err
}

bSemver, err := semver.Parse(b.Version)
if err != nil {
return false, err
}

return semverCompare(aSemver, bSemver, sensitivity)
case Alpha:
// Alphanumeric comparison (basic string compare)
Expand Down

0 comments on commit e413356

Please sign in to comment.