Automattic / Automattic/VIP-Coding-Standards

IncludingFile: Support runtime setting for custom variables in paths.

オープン
#466 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
Type: Enhancement
主要言語
PHP
スター
261
フォーク
44
平均マージ
19分
マージ済み PR(30日)
1

説明

## What problem would the enhancement address for VIP?

When a file includes a load of other includes/requires, then part of the path might be assigned to a variable, to make code DRY.

e.g.

```
$my_path = 'path/to/includes/;
require $my_path . 'foo.php';
require $my_path . 'bar.php';
require $my_path . 'baz.php';
```

The [IncludingFileSniff](https://github.com/Automattic/VIP-Coding-Standards/blob/develop/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php) checks the path, and other than a few exclusions for common functions and constants, it throws warnings for any other constants, functions and variables.

Since the prefix is often consistent for the file, the bot will add comment for each line.

The arrays of known constants and functions are all in public properties, so they can be overridden (though not implemented in the best way - see https://github.com/Automattic/VIP-Coding-Standards/issues/234), but variables cannot.

## Describe the solution you'd like

How about we made the sniff look for a runtime setting comment that defined that a particular variable was OK / should be ignored for file inclusion?
It would be set it at the top before all the includes, do the includes, then unset it again after.

```php
$my_path = 'path/to/includes/;

// phpcs:set WordPressVIPMinimum.Files.IncludingFile custom_variables[] $my_path
require $my_path . 'foo.php';
require $my_path . 'bar.php';
require $my_path . 'baz.php';
// phpcs:set WordPressVIPMinimum.Files.IncludingFile custom_variables[]
```

The bot workflow should be able to follow it since it would be an inline comment, not in a PHPCS config file.

The same could already be done with the public properties for known functions and constants, but as can be seen from the snippet above, the ideal unsetting would need to include the current values, which is awkward. As such, public `custom*` properties that are merged with the existing properties would be better.

## What code should be reported as a violation?

No new code.

## What code should *not* be reported as a violation?

Same as example above, where a runtime setting matches the name of a variable.

## Anything else

Unit tests should also cover items like `require $my_path . $my_other_path . 'baz.php';` where there are multiple variables, or `require MY_CONSTANT . $my_path . 'baz.php';` where there are multiple custom items.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php から始めて、ランタイム設定と既存の既知の定数、関数、変数がどのように扱われているかを追跡します。1つまたは複数のカスタム変数、および定数との組み合わせを対象とするユニットテストのカバレッジを追加し、文書化されたランタイム設定によって一致する include パスを許可しつつ、他の変数は違反のままにします。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
php
領域
tooling
issue の種類
機能追加
難易度
4/5
見積もり時間
3〜5日
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
38/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。