Skip to content

Escape . in regex#205

Open
mattstauffer wants to merge 2 commits into
3.xfrom
mes/fix-ruleset-regex
Open

Escape . in regex#205
mattstauffer wants to merge 2 commits into
3.xfrom
mes/fix-ruleset-regex

Conversation

@mattstauffer

Copy link
Copy Markdown
Member

The bug, says Claude: PHPCS converts rules to regex by replacing only *.*, leaving literal . as the regex any-char metachar. So */*.css in standards/Tighten/ruleset.xml:7becomes the case-insensitive regex ``./..cssi. That matches any path containing a `/` followed somewhere later by `[anychar]css`.

MetricsService.php, which is what I was seeing this bug in, lowercased, contains icss (the i + c + s + uppercase S, chars 4-7), which matches .css. So the file got excluded by the rule meant to skip .css files. Same trick excludes files like abcss.php, csServiceProvider.php, etc.

This fix should update these exclude patterns to match what we originally intended. 🤞

<!-- PSR1 2.3 Side Effects -->
<rule ref="PSR1.Files.SideEffects">
<!-- Disable side effects for index file and tests -->
<exclude-pattern>/public/index.php</exclude-pattern>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on this change would we also want to tweak things here like so?

Suggested change
<exclude-pattern>/public/index\.php</exclude-pattern>

@gcavanunez

Copy link
Copy Markdown
Contributor

I checked this branch against a fresh Laravel app using Duster 3.4.5. I generated an untracked app/Services/MetricsService.php as the only dirty PHP file and added an intentional env() call that Duster should reject.

Reproduction steps
laravel new laravel-duster-dirty-bug-reproduction \
    --git \
    --branch=main \
    --database=sqlite \
    --pest \
    --no-node \
    --no-boost \
    --no-interaction
composer require tightenco/duster:3.4.5 --dev
php artisan make:class Services/MetricsService --no-interaction

Add an env() call to the generated class so there is a violation that
PHP_CodeSniffer should report:

<?php

namespace App\Services;

class MetricsService
{
    /**
     * Create a new class instance.
     */
    public function __construct()
    {
        //
    }

    public function enabled(): bool
    {
        return env('METRICS_ENABLED', false);
    }
}

On v3.4.5, vendor/bin/duster lint --dirty failed without checking the file:

=> Linting using TLint
>> success: No issues found.
=> Linting using PHP_CodeSniffer
ERROR: No files were checked.
All specified files were excluded or did not match filtering rules.

=> Linting using PHP CS Fixer

Running this branch against the same dirty file checked 1 / 1 files and correctly reported:

The use of function env() is forbidden; use config() instead

So I think this branch hopefully resolves things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants