Automattic / Automattic/VIP-Coding-Standards

IncludingFile: Support runtime setting for custom variables in paths.

Abierto
#466 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Type: Enhancement
Lenguaje dominante
PHP
Estrellas
261
Forks
44
Merge medio
19 min
PR fusionados (30 d)
1

Descripción

## 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.

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

Comienza por WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php y rastrea cómo se gestionan la configuración de runtime y las constantes, funciones y variables conocidas existentes. Añade cobertura de pruebas unitarias para una o varias variables personalizadas y combinaciones con una constante, de modo que la configuración de runtime documentada permita las rutas de include coincidentes, mientras que las demás variables sigan siendo infracciones.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
php
Área
tooling
Tipo de issue
Nueva funcionalidad
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
38/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.