feat: add safer CLI cleanup modes#479
Open
Goryudyuma wants to merge 3 commits into
Open
Conversation
Contributor
Super-linter summary
Super-linter detected linting errors For more information, see the GitHub Actions workflow run Powered by Super-linter GO_MODULESJSCPD |
Contributor
Super-linter summary
Super-linter detected linting errors For more information, see the GitHub Actions workflow run Powered by Super-linter GO_MODULESJSCPD |
Contributor
Super-linter summary
All files and directories linted successfully For more information, see the GitHub Actions workflow run Powered by Super-linter |
massongit
reviewed
May 16, 2026
| return strings.Join(*f, ",") | ||
| } | ||
|
|
||
| func (f *stringSliceFlag) Set(value string) error { |
Comment on lines
+72
to
+74
| fmt.Fprintln(os.Stderr, err) | ||
| fmt.Fprintln(os.Stderr) | ||
| fmt.Fprint(os.Stderr, usage()) |
| } | ||
|
|
||
| if err := run(cfg); err != nil { | ||
| fmt.Fprintln(os.Stderr, err) |
| return false, errors.Wrap(diags, "parse terraform file") | ||
| } | ||
|
|
||
| ret := tfrbac(file.Body()).Bytes() |
Member
There was a problem hiding this comment.
これより前に file のnilチェックを入れた方が良さそうです。
massongit
reviewed
May 16, 2026
| } | ||
|
|
||
| if showVersion { | ||
| fmt.Fprintln(os.Stdout, version) |
Member
There was a problem hiding this comment.
Suggested change
| fmt.Fprintln(os.Stdout, version) | |
| println(version) |
println で良さそう。
massongit
reviewed
May 16, 2026
| const terraformDir = "./" //TODO: あとで引数で弄れるようにしたい | ||
| func parseConfig(args []string, stdout io.Writer) (cfg config, showVersion bool, parseErr error) { | ||
| cfg = config{ | ||
| excludeDirs: append([]string(nil), defaultExcludedDirs...), |
Member
There was a problem hiding this comment.
Suggested change
| excludeDirs: append([]string(nil), defaultExcludedDirs...), | |
| excludeDirs: append([]string{}, defaultExcludedDirs...), |
個人的にはこの方が好みかも。
| cfg.stdout = io.Discard | ||
| } | ||
| if len(cfg.excludeDirs) == 0 { | ||
| cfg.excludeDirs = append([]string(nil), defaultExcludedDirs...) |
Member
There was a problem hiding this comment.
Suggested change
| cfg.excludeDirs = append([]string(nil), defaultExcludedDirs...) | |
| cfg.excludeDirs = append([]string{}, defaultExcludedDirs...) |
個人的にはこの方が好みかも。
massongit
reviewed
May 16, 2026
Comment on lines
+117
to
+125
| if cfg.stdout == nil { | ||
| cfg.stdout = io.Discard | ||
| } | ||
| if len(cfg.excludeDirs) == 0 { | ||
| cfg.excludeDirs = append([]string(nil), defaultExcludedDirs...) | ||
| } | ||
| if len(cfg.targets) == 0 { | ||
| cfg.targets = []string{"."} | ||
| } |
Member
There was a problem hiding this comment.
この辺は parseConfig と処理が被っている感じがあるので、この辺は削除するか、初期化のメソッドを用意して、それをここと parseConfig から呼び出す形にすると良さそう。
massongit
reviewed
May 16, 2026
Comment on lines
-140
to
-144
| i++ // 後ろに改行がある場合はそれを削除 | ||
| i++ | ||
| } else if endTokenPos-2 > startTokenPos && | ||
| tokens[endTokenPos-1].Type == hclsyntax.TokenNewline && | ||
| tokens[endTokenPos-2].Type == hclsyntax.TokenNewline { | ||
| endTokenPos-- // 後ろに改行はないけど、上に二つ以上改行がある場合、一つ削除 |
massongit
reviewed
May 16, 2026
Comment on lines
+337
to
+351
| wf, err := root.OpenFile(relPath, os.O_WRONLY|os.O_TRUNC, perm) | ||
| if err != nil { | ||
| return errors.Wrap(err, "open file for write") | ||
| } | ||
| defer func() { | ||
| if closeErr := wf.Close(); closeErr != nil { | ||
| err = errors.Join(err, errors.Wrap(closeErr, "close file")) | ||
| } | ||
| }() | ||
|
|
||
| if _, err := wf.Write(src); err != nil { | ||
| return errors.Wrap(err, "write file") | ||
| } | ||
|
|
||
| return nil |
Comment on lines
+288
to
+298
| src, err := readTFFile(root, relPath) | ||
| if err != nil { | ||
| return false, errors.Wrap(err, "read terraform file") | ||
| } | ||
|
|
||
| file, diags := hclwrite.ParseConfig(src, displayPath, hcl.InitialPos) | ||
| if diags.HasErrors() { | ||
| return false, errors.Wrap(diags, "parse terraform file") | ||
| } | ||
|
|
||
| ret := tfrbac(file.Body()).Bytes() |
Comment on lines
+201
to
+204
| relPath, relErr := filepath.Rel(target, filePath) | ||
| if relErr != nil { | ||
| return errors.Wrap(relErr, "build relative path") | ||
| } |
Comment on lines
+188
to
190
| if !strings.HasSuffix(entry.Name(), ".tf") { | ||
| return nil | ||
| } |
Comment on lines
177
to
179
| if walkErr != nil { | ||
| return errors.Wrap(walkErr, "Error on filepath.Walk") | ||
| return errors.Wrap(walkErr, "walk directory") | ||
| } |
Comment on lines
+166
to
+174
| root, err := os.OpenRoot(target) | ||
| if err != nil { | ||
| return errors.Wrap(err, "Error on os.OpenRoot") | ||
| return 0, errors.Wrap(err, "open root") | ||
| } | ||
| defer func(root *os.Root) { | ||
| defer func() { | ||
| if closeErr := root.Close(); closeErr != nil { | ||
| err = errors.Join(err, errors.Wrap(closeErr, "Error on Close")) | ||
| resultErr = errors.Join(resultErr, errors.Wrap(closeErr, "close root")) | ||
| } | ||
| }(root) | ||
| }() |
massongit
reviewed
May 16, 2026
Comment on lines
+89
to
+90
| func parseConfig(args []string, stdout io.Writer) (cfg config, showVersion bool, parseErr error) { | ||
| cfg = config{ |
Member
There was a problem hiding this comment.
Suggested change
| func parseConfig(args []string, stdout io.Writer) (cfg config, showVersion bool, parseErr error) { | |
| cfg = config{ | |
| func parseConfig(args []string, stdout io.Writer) (config, bool, error) { | |
| cfg := config{ |
戻り値の名前は定義しなくて良い気がする。
massongit
reviewed
May 16, 2026
Comment on lines
+239
to
+247
| root, err := os.OpenRoot(filepath.Dir(target)) | ||
| if err != nil { | ||
| return 0, errors.Wrap(err, "open file root") | ||
| } | ||
| defer func() { | ||
| if closeErr := root.Close(); closeErr != nil { | ||
| err = errors.Join(err, errors.Wrap(closeErr, "close file root")) | ||
| } | ||
| }() |
massongit
reviewed
May 16, 2026
| }() | ||
|
|
||
| err = filepath.Walk(terraformDir, func(filePath string, info os.FileInfo, walkErr error) (err error) { | ||
| err = filepath.WalkDir(target, func(filePath string, entry fs.DirEntry, walkErr error) error { |
Member
There was a problem hiding this comment.
filepath と filePath は紛らわしいので、 filePath を変えても良いかも。
参考: dev-hato/hato-bot-go#363
| return errors.Wrap(err, "Error on readTFFile") | ||
| pathKey, pathErr := uniquePathKey(filePath) | ||
| if pathErr != nil { | ||
| return pathErr |
Member
There was a problem hiding this comment.
Suggested change
| return pathErr | |
| return errors.Wrap(pathErr, "uniquePathKey") |
pathErr をWrapしても良さそう。
| return errors.Wrap(err, "Error on Write") | ||
| changed, processErr := processTFFile(root, relPath, filePath, info.Mode().Perm(), cfg) | ||
| if processErr != nil { | ||
| return processErr |
Member
There was a problem hiding this comment.
Suggested change
| return processErr | |
| return errors.Wrap(processErr, "processTFFile") |
processErr をWrapしても良さそう。
| pathKey, err := uniquePathKey(target) | ||
| if err != nil { | ||
| return errors.Wrap(err, "Error walking through Terraform directory") | ||
| return 0, err |
Member
There was a problem hiding this comment.
Suggested change
| return 0, err | |
| return 0, errors.Wrap(err, "uniquePathKey") |
uniquePathKey をWrapしても良さそう。
|
|
||
| changed, err := processTFFile(root, filepath.Base(target), target, perm, cfg) | ||
| if err != nil { | ||
| return 0, err |
Member
There was a problem hiding this comment.
Suggested change
| return 0, err | |
| return 0, errors.Wrap(err, "processTFFile") |
processTFFile をWrapしても良さそう。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
codexに「いい感じにして」って投げたらこうなったんだが、あってるんだろうか・・・