Fix invalid 'implicit scope' report on global or autoload function

[rhysd]

関数名にもスコープを明示しろという警告が出てしまう問題を修正しました．

function SomeFunc()
endfunction

function aaa#bbb()
endfunction

実行結果：

foo.vim:1:10: Make the scope explicit like `g:SomeFunc` (see Anti-pattern of vimrc (Scope of identifier))
foo.vim:4:10: Make the scope explicit like `g:aaa#bbb` (see Anti-pattern of vimrc (Scope of identifier))

Vim script 的には関数名に g: をつけるとエラーになる（昔は通ってしまっていた）はずです．

まだテストが追加できていないのでマージできる状態ではないです．
修正が正しいかどうか（Python よく知らないので変な修正になってるかも…）をレビューお願いします．問題無さそうならテスト足してみます．

直りそうな issue: #135

[landscape-bot]

Code quality remained the same when pulling f53a988 on rhysd:fix-ProhibitImplicitScopeVariable into 6fcf081 on Kuniwak:master.

[thinca]

Vim script 的には関数名に g: をつけるとエラーになる

今は、ユーザ定義のグローバル関数に g: をつけると、ついていないのと全く同じ扱いになります。
エラーにはなりませんが、付けない場合が多いので、lint で検出するべきではないと思います。

[rhysd]

@thinca

すみません，説明間違ってますね．指摘ありがとうございます．
多分 g:Func みたいなのを想像していると思うのですが，例えば vital#some_func みたいな関数にも 「g:vital#some_func にしろ」というエラーメッセージが出てしまうのが現状です．
（結論としては変わらず，エラーを出すべきではない，です．）

[Kuniwak]

返答が遅れてすみません 😷
autoload については #129 で警告を抑止するオプションがつきました（ただし、コメントからの設定ができないという問題が残っています）。

エラーにはなりませんが、付けない場合が多いので、lint で検出するべきではないと思います。

これについてですが、#128 で似たようなことを議論しました。
私の方針は、「デフォルトは警告して、設定で警告を抑制できるようにする」です。

この理由は以下の通りです：

1. スコープを明示させることで、初心者がスコープを学習できる

2. パワーユーザーにとっては煩い限りなので、設定で警告を抑制できるようにする

   （デフォルトで警告しないのも考えられなくはないですが、初心者とパワーユーザーのどちらにこの設定を強いることになるかを考えたとき、初心者に強いるのは酷だろうという判断です）

[thinca]

ユーザ定義関数については、本来大文字で開始して g: を付けないのが正しいのですが、変数のルールと混同して誤って g: を付けて書いていたユーザーも少なからずいたようです。
以前の Vim では Vim script の関数名のチェックがざるで、関数名の中のどこに : が含まれていても関数名として正しく扱われてしまっていたのですが、Vim 7.4.260 からチェックが厳密になり、先頭の s: 以外の : は一切許可されなくなりました。
ただ、最初に挙げた事情があったため、7.4.265 で、g: については g: がなかったように扱うパッチが入りました。
なので、g: は付けられるけれども、付けるのは本来誤りです。ドキュメントの関数定義の部分(:help E124) にも、許されるのは s: のみで、b: や g: は許可されないと明記されています。

なので、g: なしを lint で標準でエラーにするのはよろしくない、と言うのが私の考えです。(なんなら g: が付いてたら消すようにサジェストしてもいいくらい)

初心者にとっては Vim のルール自体がわかりづらく映ってしまいそうという懸念はありますが、結局はそういうものなので、そういうものとして覚えて欲しい、むしろ間違った書き方を覚えられてしまう方がつらいかな、と思います。

[Kuniwak]

なるほど、そのような背景を知りませんでした。
こうなると、むしろ g: をつけたときに警告するほうが望ましいですね（大文字始まりの関数名にするよう警告する感じでしょうか）。

[thinca]

そうですね。それが良いと思います。

[Kuniwak]

ありがとうございます！
#136 (comment) は別PRでやりましょう。

この修正を次リリースに含めます。

[Kuniwak]

@rhysd テストを追加していただければ merge します！よろしくお願いします。
（返信が遅れて申し訳ありませんでした 🙇）

[rhysd]

👍

[rhysd]

ぬ，この PR は取り込む方向でいきますか？
それならテストケース追加とか明日やってみます

[Kuniwak]

はい、取り込む方針です。
取り込む理由は #136 (comment) です。

[rhysd]

了解です．とりあえず rebase して master に追いつかせました．

[landscape-bot]

Code quality remained the same when pulling 2676a92 on rhysd:fix-ProhibitImplicitScopeVariable into 18217d7 on Kuniwak:master.

[landscape-bot]

Code quality remained the same when pulling f5acb44 on rhysd:fix-ProhibitImplicitScopeVariable into 18217d7 on Kuniwak:master.

[rhysd]

2676a92 で警告を出ないようにしたパターンについてテストケースを修正しました．ちゃんと意図通りテストが通るのを確認済みです．

[rhysd]

g: をつけていない関数名が valid であることを確認するテストケースを追加しました．また，よくあるミスとして意図せず for 文の制御変数がグローバル変数として定義されてしまっているパターンをテストケースに追加しました．

[rhysd]

@Kuniwak

これで一通り作業が完了したつもりです．

[landscape-bot]

Code quality remained the same when pulling 97d024d on rhysd:fix-ProhibitImplicitScopeVariable into 18217d7 on Kuniwak:master.

[Kuniwak]

This file should collect only invalid cases.
So, https://github.com/rhysd/vint/blob/fix-ProhibitImplicitScopeVariable/test/fixture/policy/prohibit_implicit_scope_variable_invalid.vim#L10-L19 should be removed from the file (and it should moved to prohibit_implicit_scope_variable_valid.vim, but it was already done. thanks 😃 ).

[Kuniwak]

ありがとうございます！
ひとつだけコメントした点以外は 💯 です。
よろしくお願いします！

[rhysd]

レビューありがとうございます．指摘いただいた余分なテストケースを削除しました．

[Kuniwak]

ありがとうございます！ 👍