Automattic / Automattic/VIP-Coding-Standards

IncludingFile: Support runtime setting for custom variables in paths.

未关闭
#466 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
Type: Enhancement
主要语言
PHP
星标
261
派生
44
平均合并
19 分钟
30 天内合并 PR
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 开始,跟踪运行时设置以及现有的已知常量、函数和变量是如何处理的。为一个或多个自定义变量以及与常量的组合添加单元测试覆盖,并使文档所述的运行时设置允许匹配的 include 路径,而其他变量仍然属于违规项。

由索引模型根据 Issue 内容生成。

评估

技术栈
php
领域
tooling
Issue 类型
功能
难度
4/5
预计耗时
3-5 天
活跃度
停滞
描述清晰度
基本清楚
新手友好度
38/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。