Skip to content

Conversation

usatie
Copy link
Contributor

@usatie usatie commented Mar 17, 2023

#8 のissueを修正するコードを書いてみました。

Copy link
Member

@tenntenn tenntenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

テストを追加してもらえると嬉しいです。

@@ -50,6 +53,14 @@ func WithModules(t *testing.T, srcdir string, modfile io.Reader) (dir string) {
}

for _, file := range files {
// Prepend line directive to .go files
if strings.HasSuffix(file.Name(), ".go") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filepath.Extを使ってください

// Prepend line directive to .go files
if strings.HasSuffix(file.Name(), ".go") {
fn := filepath.Join(path, file.Name())
originalFn := strings.TrimPrefix(fn, dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

やりたいことはfilepath.Baseでしょうか?

fn := filepath.Join(path, file.Name())
originalFn := strings.TrimPrefix(fn, dir)
if err := prependToFile(fn, fmt.Sprintf("//line %s:1\n", originalFn)); err != nil {
t.Fatal("cannot prepend.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

もう少し理由をわかりやすく書いてもらえると嬉しいです。

@@ -85,6 +96,50 @@ func WithModules(t *testing.T, srcdir string, modfile io.Reader) (dir string) {
return dir
}

func appendFileContent(tmp io.Writer, filename string) error {
f, err := os.OpenFile(filename, os.O_RDONLY, 0600)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.Openを使わない理由はありますか?

if strings.HasSuffix(file.Name(), ".go") {
fn := filepath.Join(path, file.Name())
originalFn := strings.TrimPrefix(fn, dir)
if err := prependToFile(fn, fmt.Sprintf("//line %s:1\n", originalFn)); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmt.Sprintf使うなら直接fmt.Fprintlnでファイルに出力したら良いと思いました。

return nil
}

func prependToFile(filename string, content string) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.Removeはdeferですれば良さそうです。

}
// Write the original file content
if err := appendFileContent(tmp, filename); err != nil {
tmp.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

エラー処理が必要です。

// Write the original file content
if err := appendFileContent(tmp, filename); err != nil {
tmp.Close()
os.Remove(tmpFilePath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

エラー処理が必要です。

k1LoW added a commit to k1LoW/testutil that referenced this pull request Sep 26, 2023
k1LoW added a commit to k1LoW/testutil that referenced this pull request Sep 26, 2023
@tenntenn tenntenn merged commit f152afd into gostaticanalysis:main Mar 1, 2024
@github-actions github-actions bot mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants