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

Phpcs option for auto fixing or prompt to fix #663

Closed
hkirsman opened this issue Aug 19, 2019 · 3 comments
Closed

Phpcs option for auto fixing or prompt to fix #663

hkirsman opened this issue Aug 19, 2019 · 3 comments

Comments

@hkirsman
Copy link

hkirsman commented Aug 19, 2019

Q A
Version 0.15.2
Bug? no
New feature? yes

I think current way of printing out command to auto fix is a bit cumbersome:

You can fix some errors by running following command:
'vendor/bin/phpcbf' '--standard=phpcs.xml' '--encoding=utf-8' '--report=full' '--report-width=120' '--ignore=cfg/,libraries/' 'code/sites/all/modules/custom/site/site.module'

Instead for example I have bash script that prompts for y/n if Codesniffer finds issues and in the output it says that some of the issues can be fixed automatically. If you press y, then phpcbf is ran with the same rules as was phpcs.
Other option would be to add some parameter in the config to always automatically fix the issues if possible.
We have really legacy code and it has too many issues. Copy pasting is an extra step if know we want to run phpcbf always.
Could we implement something like that?

Here's part of the code to do that:

      # Fix the violations automatically if possible.
      if cat "$phpcs_log" | grep -s 'PHPCBF CAN FIX'; then
        # Allows us to read user input below, assigns stdin to keyboard
        exec < /dev/tty
    
        read -p "$(echo -e "\n${GREEN}Fix these violations automatically? (y/n)${DEFAULT_COLOR} ")" yn
        if [ "$yn" = "y" ]; then

PHP equivalent package is here: https://github.com/hkirsman/code-quality-phpcs/blob/master/bin/phpcs_phpcbf

This unfortunately does not work if another scripts calls it as prompt will just continue without waiting for input. I think this could work if GrumpPHP would execute prompt in main thread.

My configuration

# grumphp.yml
parameters:
  tasks:
    phpcs:
      standard: phpcs.xml
      report_width: 120
      encoding: utf-8
      ignore_patterns:
        - cfg/
        - libraries/
      triggered_by:
        - php
        - module
        - inc

phpcs.xml (just because it's in my setup, nothing important related to the issue):

<?xml version="1.0"?>
<!-- See http://pear.php.net/manual/en/package.php.php-codesniffer.annotated-ruleset.php -->
<ruleset name="Relax Drupal">
    <description>Drupal coding standard</description>

    <!-- exclude some Drupal files that contain issues -->
    <exclude-pattern>*default.settings.php</exclude-pattern>

    <!-- exclude minified files -->
    <exclude-pattern>*.min.*</exclude-pattern>

    <!-- exclude third-party node modules -->
    <exclude-pattern>node_modules/</exclude-pattern>

    <!-- exclude CSS files, where we don't usually follow Drupal standards  -->
    <exclude-pattern>*.css</exclude-pattern>

    <rule ref="vendor/drupal/coder/coder_sniffer/Drupal">
        <exclude name="Drupal.Files.TxtFileLineLength" />
        <exclude name="Drupal.InfoFiles.AutoAddedKeys" />
    </rule>

    <!-- force short array notation - []  -->
    <rule ref="Generic.Arrays.DisallowLongArraySyntax.Found">
        <type>error</type>
    </rule>

    <rule ref="Drupal.Files.LineLength">
        <properties>
            <!-- Be a bit more tolerant when it comes to line lengths -->
            <property name="lineLimit" value="110"/>
        </properties>
        <type>error</type>
    </rule>

</ruleset>
@veewee
Copy link
Contributor

veewee commented Aug 19, 2019

Hi @hkirsman,

Thanks for reporting.
We won't support autofixing at this point.

Please check:
https://github.com/phpro/grumphp/blob/master/doc/faq.md#does-grumphp-support-automatic-fixing

I suggest you run phpcbf once on the complete codebase if you want to fix the code style everywhere.
Alternatively, you can use / create an extenstion that allows autofixing the codebase.

@veewee veewee closed this as completed Aug 19, 2019
@hkirsman
Copy link
Author

@veewee I understand the philosophy but this particular feature would only run the command if you would say yes. Then you would need to still git add and review the changes. How is that different from adding command at the end of the report that you can copy/paste and fix the code? :)

@veewee
Copy link
Contributor

veewee commented Aug 22, 2019

Haven't thought about it that way. Fixing code AFTER all validations ran might be a nice addition.
We are planning to do some changes on the task and task runnen system later this year.
That might be a better time for rethinking about it.

@veewee veewee mentioned this issue Feb 14, 2020
40 tasks
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

2 participants