Skip to content

Commit

Permalink
remove redundant arguments (#11)
Browse files Browse the repository at this point in the history
* remove redundant arguments

* use var

* fix dl3025

* fix dl3001

* fix dl3003

same line errors

* fix error message dl3000, value dl3008
  • Loading branch information
zabio3 committed Jan 27, 2020
1 parent dfa4165 commit b910596
Show file tree
Hide file tree
Showing 65 changed files with 124 additions and 124 deletions.
2 changes: 1 addition & 1 deletion cmd/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Other Commands:
},
{
command: "godolint ../testdata/DL3001_Dockerfile",
expectedOutStream: "#6 DL3001 For some bash commands it makes no sense running them in a Docker container like `ssh`, `vim`, `shutdown`, `service`, `ps`, `free`, `top`, `kill`, `mount`, `ifconfig`. \n",
expectedOutStream: "#6 DL3001 For some bash commands it makes no sense running them in a Docker container like `free`, `ifconfig`, `kill`, `mount`, `ps`, `service`, `shutdown`, `ssh`, `top`, `vim`. \n",
expectedErrStream: "",
expectedExitCode: ExitCodeOK,
},
Expand Down
5 changes: 3 additions & 2 deletions linter/rules/dl3000.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rules

import (
"fmt"
"path/filepath"

"github.com/moby/buildkit/frontend/dockerfile/parser"
Expand All @@ -12,10 +13,10 @@ func validateDL3000(node *parser.Node) (rst []ValidateResult, err error) {
if child.Value == WORKDIR {
absPath, err := filepath.Abs(child.Next.Value)
if err != nil {
return nil, err
return nil, fmt.Errorf("#%v DL3000: failed to convert relative path to absolute path (err: %s)", child.StartLine, err)
}
if absPath != child.Next.Value {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3000_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ ADD . /go
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 3, addMsg: ""},
{line: 3},
},
expectedErr: nil,
},
Expand Down
17 changes: 12 additions & 5 deletions linter/rules/dl3001.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,25 @@ import (
)

// validateDL3001 is "For some bash commands it makes no sense running them in a Docker container
// like ssh, vim, shutdown, service, ps, free, top, kill, mount, ifconfig."
// like free, ifconfig, kill, mount, ps, service, shutdown, ssh, top, vim"
func validateDL3001(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == RUN {
for _, v := range strings.Fields(child.Next.Value) {
for _, c := range []string{"ssh", "vim", "shutdown", "service", "ps", "free", "top", "kill", "mount"} {
if v == c {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
}
if existsTools(v) {
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
}
}
return rst, nil
}

func existsTools(s string) bool {
for _, c := range []string{"free", "ifconfig", "kill", "mount", "ps", "service", "shutdown", "ssh", "top", "vim"} {
if s == c {
return true
}
}
return false
}
2 changes: 1 addition & 1 deletion linter/rules/dl3001_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ RUN top
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 6, addMsg: ""},
{line: 6},
},
expectedErr: nil,
},
Expand Down
9 changes: 4 additions & 5 deletions linter/rules/dl3002.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,20 @@ import (

// validateDL3002 is "Last user should not be root."
func validateDL3002(node *parser.Node) (rst []ValidateResult, err error) {
isLastRootUser := false
var isLastRootUser bool
var lastRootUserPos int
for _, child := range node.Children {
if child.Value == USER {
if child.Next.Value == "root" || child.Next.Value == "0" {
isLastRootUser = true
lastRootUserPos = child.StartLine
} else {
isLastRootUser = false
lastRootUserPos = 0
continue
}
isLastRootUser = false
}
}
if isLastRootUser {
rst = append(rst, ValidateResult{line: lastRootUserPos, addMsg: ""})
rst = append(rst, ValidateResult{line: lastRootUserPos})
}
return rst, nil
}
2 changes: 1 addition & 1 deletion linter/rules/dl3002_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ADD . /go
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 3, addMsg: ""},
{line: 3},
},
expectedErr: nil,
},
Expand Down
3 changes: 2 additions & 1 deletion linter/rules/dl3003.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ func validateDL3003(node *parser.Node) (rst []ValidateResult, err error) {
if child.Value == RUN {
for _, v := range strings.Fields(child.Next.Value) {
if v == "cd" {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
break
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3003_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ RUN cd /usr/src/app && git clone git@github.com:zabio3/godolint.git /usr/src/app
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 6, addMsg: ""},
{line: 6},
},
expectedErr: nil,
},
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3004.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func validateDL3004(node *parser.Node) (rst []ValidateResult, err error) {
if child.Value == RUN {
for _, v := range strings.Fields(child.Next.Value) {
if v == "sudo" {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
break
}
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3004_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ ADD . /go
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 3, addMsg: ""},
{line: 3},
},
expectedErr: nil,
},
Expand Down
4 changes: 2 additions & 2 deletions linter/rules/dl3005.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
func validateDL3005(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == RUN {
isAptGet, isUpgrade := false, false
var isAptGet, isUpgrade bool
for _, v := range strings.Fields(child.Next.Value) {
switch v {
case "apt-get":
Expand All @@ -20,7 +20,7 @@ func validateDL3005(node *parser.Node) (rst []ValidateResult, err error) {
}
}
if isAptGet && isUpgrade {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3005_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ADD . /go
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 2},
},
expectedErr: nil,
},
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3006.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var regexDL3006 = regexp.MustCompile(`.+[:].+`)
func validateDL3006(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == FROM && !regexDL3006.MatchString(child.Next.Value) {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
return rst, nil
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3006_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ADD . /go
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 1, addMsg: ""},
{line: 1},
},
expectedErr: nil,
},
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3007.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var regexDL3007 = regexp.MustCompile(`.*:latest`)
func validateDL3007(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == FROM && regexDL3007.MatchString(child.Next.Value) {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
return rst, nil
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3007_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ADD . /go
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 1, addMsg: ""},
{line: 1},
},
expectedErr: nil,
},
Expand Down
7 changes: 4 additions & 3 deletions linter/rules/dl3008.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ var regexDL3008 = regexp.MustCompile(`.+=.+`)
func validateDL3008(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == RUN {
isAptGet, isInstall, length := false, false, len(rst)
var isAptGet, isInstall bool
l := len(rst)
for _, v := range strings.Fields(child.Next.Value) {
switch v {
case "apt-get":
Expand All @@ -26,8 +27,8 @@ func validateDL3008(node *parser.Node) (rst []ValidateResult, err error) {
isAptGet, isInstall = false, false
continue
default:
if isInstall && !regexDL3008.MatchString(v) && length == len(rst) {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
if isInstall && !regexDL3008.MatchString(v) && l == len(rst) {
rst = append(rst, ValidateResult{line: child.StartLine})
isAptGet, isInstall = false, false
}
}
Expand Down
4 changes: 2 additions & 2 deletions linter/rules/dl3008_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ RUN apt-get install python
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 2},
},
expectedErr: nil,
},
Expand All @@ -28,7 +28,7 @@ RUN apt-get install python && apt-get clean
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 2},
},
expectedErr: nil,
},
Expand Down
4 changes: 2 additions & 2 deletions linter/rules/dl3009.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import (
func validateDL3009(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == RUN && isDL3009Error(child) {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
return rst, nil
}

func isDL3009Error(node *parser.Node) bool {
isAptGet, isInstalled, hasClean, isRm, hasRemove := false, false, false, false, false
var isAptGet, isInstalled, hasClean, isRm, hasRemove bool
for _, v := range strings.Fields(node.Next.Value) {
isAptGet, isInstalled, hasClean, isRm, hasRemove = updateDL3009Status(v, isAptGet, isInstalled, hasClean, isRm, hasRemove)
}
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3009_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ RUN apt-get update && apt-get install -y python
CMD ["go", "run", "main.go"]
`,
expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 2},
},
expectedErr: nil,
},
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3010.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func validateDL3010(node *parser.Node) (rst []ValidateResult, err error) {
if child.Value == COPY {
args := strings.Fields(child.Next.Value)
if len(args) >= 1 && isCompressionExt.MatchString(args[0]) {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3010_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ COPY hoge.tar.xz /
CMD ["go", "run", "main.go"]
`, expectedRst: []ValidateResult{
{line: 3, addMsg: ""},
{line: 3},
},
expectedErr: nil,
},
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3011.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func validateDL3011(node *parser.Node) (rst []ValidateResult, err error) {
return nil, fmt.Errorf("#%v DL3011 not numeric is the value set for the port: %s", child.StartLine, port.Value)
}
if portNum < 0 || portNum > 65535 {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion linter/rules/dl3011_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestValidateDL3011(t *testing.T) {
EXPOSE 80000
`, expectedRst: []ValidateResult{
{line: 3, addMsg: ""},
{line: 3},
},
expectedErr: nil,
},
Expand Down
5 changes: 3 additions & 2 deletions linter/rules/dl3013.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ var regexVersion3013 = regexp.MustCompile(`.+[=|@].+`)
func validateDL3013(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == RUN {
isPip, isInstall, length := false, false, len(rst)
var isPip, isInstall bool
length := len(rst)
for _, v := range strings.Fields(child.Next.Value) {
switch v {
case "pip":
Expand All @@ -26,7 +27,7 @@ func validateDL3013(node *parser.Node) (rst []ValidateResult, err error) {
isPip, isInstall = false, false
default:
if isPip && isInstall && !regexVersion3013.MatchString(v) && length == len(rst) {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
isPip, isInstall = false, false
}
Expand Down
6 changes: 3 additions & 3 deletions linter/rules/dl3013_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ RUN pip install django
RUN pip install https://github.com/Banno/carbon/tarball/0.9.x-fix-events-callback
`,
expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 3, addMsg: ""},
{line: 2},
{line: 3},
},
expectedErr: nil,
},
Expand All @@ -26,7 +26,7 @@ RUN pip install https://github.com/Banno/carbon/tarball/0.9.x-fix-events-callbac
RUN pip install django && pip install https://github.com/Banno/carbon/tarball/0.9.x-fix-events-callback
`,
expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 2},
},
expectedErr: nil,
},
Expand Down
5 changes: 3 additions & 2 deletions linter/rules/dl3014.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ var yesPattern = regexp.MustCompile(`^-(y|-yes|-assume-yes)$`)
func validateDL3014(node *parser.Node) (rst []ValidateResult, err error) {
for _, child := range node.Children {
if child.Value == RUN {
isAptGet, isInstalled, length := false, false, len(rst)
var isAptGet, isInstalled bool
length := len(rst)
for _, v := range strings.Fields(child.Next.Value) {
switch v {
case "apt-get":
Expand All @@ -26,7 +27,7 @@ func validateDL3014(node *parser.Node) (rst []ValidateResult, err error) {
isAptGet, isInstalled = false, false
default:
if isInstalled && !yesPattern.MatchString(v) && length == len(rst) {
rst = append(rst, ValidateResult{line: child.StartLine, addMsg: ""})
rst = append(rst, ValidateResult{line: child.StartLine})
}
isAptGet, isInstalled = false, false
}
Expand Down
4 changes: 2 additions & 2 deletions linter/rules/dl3014_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ func TestValidateDL3014(t *testing.T) {
RUN apt-get install python=2.7
`,
expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 2},
},
expectedErr: nil,
},
{
dockerfileStr: `FROM debian
RUN apt-get install python=2.7 && apt-get install ruby
`, expectedRst: []ValidateResult{
{line: 2, addMsg: ""},
{line: 2},
},
expectedErr: nil,
},
Expand Down

0 comments on commit b910596

Please sign in to comment.