Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scan files being erroneously scanned #35

Open
dannydover opened this issue Jul 7, 2020 · 7 comments
Open

Scan files being erroneously scanned #35

dannydover opened this issue Jul 7, 2020 · 7 comments

Comments

@dannydover
Copy link

First off, thank you for building this! This has been working great for me and is really helping my codebase stay up-to-par.

However, I am running into one reoccurring issue. About 1/4 of the time when this runs automatically after opening a pull request, I receive the following error:

🚫 Error: Filenames should be all lowercase with hyphens as word separators. Expected phpcs-scan-ysewvc.php, but found phpcs-scan-Ysewvc.php (WordPress.Files.FileName.NotHyphenatedLowercase).

"phpcs-scan-Ysewvc.php" -- As you might expect, the filename changes each time this occurs (characters after the second hyphen differ):

Is there a way to either ignore these phpcs scan files or fix the root cause of this issue?

For context, I have included my workflow file below:

# Original Documentation https://github.com/rtCamp/action-phpcs-code-review

on: pull_request

name: Code Quality
jobs:
  runPHPCSInspection:
    name: Run PHPCS linter
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
      with:
        ref: ${{ github.event.pull_request.head.sha }}
    - name: Run PHPCS inspection
      uses: rtCamp/[email protected]
      env:
        GH_BOT_TOKEN: ${{ secrets.GH_CICD_BOT }}
        SKIP_FOLDERS: "tests,.github,cypress"
      with:
        args: "WordPress,WordPress-Core,WordPress-Docs"

@dannydover dannydover changed the title Scan files being erroronously scanned Scan files being erroneously scanned Jul 7, 2020
@mrrobot47
Copy link
Member

Please refer: #34

@NickGreen
Copy link

Thanks for making this issue, I'm getting the same problem:

🚫 Error: Filenames should be all lowercase with hyphens as word separators. Expected phpcs-scan-xe6at7.php, but found phpcs-scan-xe6aT7.php (WordPress.Files.FileName.NotHyphenatedLowercase).

Is there anything I can do to help troubleshoot or resolve this?

@mrrobot47
Copy link
Member

I will be looking into this issue over the weekend or in the next week. Will loop you in by Monday or Tuesday with more details 🙂

@dannydover
Copy link
Author

@mrrobot47 Thank you! That is very much appreciated it.

@NickGreen
Copy link

NickGreen commented Jul 20, 2020

@mrrobot47 Sorry to bother you - I appreciate all the work you've done on this. I'm having problems with the workaround of ignoring the WordPress.Files.FileName.NotHyphenatedLowercase rule.

I want to use the WordPress-Extra ruleset, which includes the WordPress-Core rules, but I think I'm excluding that specific rule wrong. Here's my phpcs.xml rule declaration:

    <rule ref="WordPress-Extra">
        <exclude name="WordPress.Files.FileName.NotHyphenatedLowercase" />
        <exclude name="WordPress.Files.FileName.InvalidClassFileName" />
    </rule>

However, I'm still occasionally getting the error above, and my current theory is I have to ignore that rule specifically from the WordPress ruleset? Any ideas?

Disregard - I figured it out. The phpcs.xml file has to be included in the branch that is being tested - or it won't know to look at it. So, if you've added the xml file to the master branch, then make sure to pull it into the branch that you need to test.

@mrrobot47
Copy link
Member

Sorry for replying late on this issue. I got caught up on other work and could not check on this.

The issue of WordPress.Files.FileName sniff is due to how the temp files are created having the diff of the PR. tempnam function creates file with the name that could also contain uppercase letters. This issue needs more discussion on repo vip-go-ci, will create an issue for the same and discuss it there later. Or you can initiate the same and reference this issue.

For now, I have added the option to skip sniffs using env var: PHPCS_SNIFFS_EXCLUDE, this is the equivalent of --exclude flag of phpcs. This works in both cases, when you have phpcs.xml file in repo, or only have sniffs listed as args in the workflow file.

Update your workflow file according to the latest release usage having the PHPCS_SNIFFS_EXCLUDE env var and you will not face this error again:

on: pull_request

name: Inspections
jobs:
  runPHPCSInspection:
    name: Run PHPCS inspection
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
      with:
        ref: ${{ github.event.pull_request.head.sha }}
    - name: Run PHPCS inspection
      uses: rtCamp/[email protected]
      env:
        GH_BOT_TOKEN: ${{ secrets.GH_BOT_TOKEN }}
        SKIP_FOLDERS: "tests,.github"
        PHPCS_SNIFFS_EXCLUDE: "WordPress.Files.FileName"
      with:
        args: "WordPress,WordPress-Core,WordPress-Docs"

@NickGreen
Copy link

Sorry for the late response on this.

Update your workflow file according to the latest release usage having the PHPCS_SNIFFS_EXCLUDE env var

Is adding this required, or will the check still work if that doesn't exist in the action?

I'm currently trying to track down an issue where the exclude patterns in the phpcs.xml file are being ignored.

Possibly related, this is showing in the action output:

Invalid PHPCS sniff(s) specified in options or options file (option 'phpcs-sniffs-exclude')

Likely because PHPCS_SNIFFS_EXCLUDE: is not defined in the action.

Will not having that defined affect the exclude rules in the phpcs.xml file?

richardkorthuis added a commit to acato-plugins/wp-rest-cache that referenced this issue Feb 6, 2023
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

No branches or pull requests

3 participants