Automattic / Automattic/VIP-Coding-Standards

IncludingFile: Support runtime setting for custom variables in paths.

Offen
#466 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Type: Enhancement
Vorherrschende Sprache
PHP
Sterne
261
Forks
44
Ø Merge
19 Min.
Gemergte PRs (30 T.)
1

Beschreibung

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

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginne mit WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php und verfolge, wie Laufzeiteinstellungen sowie die vorhandenen bekannten Konstanten, Funktionen und Variablen behandelt werden. Füge Unit-Tests für eine oder mehrere benutzerdefinierte Variablen und Kombinationen mit einer Konstante hinzu, wobei die dokumentierte Laufzeiteinstellung passende Include-Pfade zulässt, während andere Variablen weiterhin Verstöße darstellen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
php
Bereich
tooling
Issue-Typ
Feature
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
38/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.