praem90 / nvim-phpcsf

A simple nvim plugin wrapper for both phpcs and phpcbf.
https://praem90.github.io/nvim-phpcsf/
13 stars 3 forks source link

Error after opening a PHP file. #9

Closed xavortm closed 3 months ago

xavortm commented 3 months ago

Hi! First, thanks for the plugin, it's really useful to me!

But unfortunately, sometimes I get an error for every PHP file I open, sometimes I don't and I am struggling to locate the problem. Here is what I get:

Screenshot 2024-06-12 at 09 46 13@2x

After I Q to close it, I can use the file and edit, but then when I save, it runs again.

This is my config file for the plugin:

return {
    -- PHPCS installation. 
    -- https://github.com/WordPress/WordPress-Coding-Standards?tab=readme-ov-file#installation
    {
        "praem90/nvim-phpcsf",
        config = function()
            vim.api.nvim_create_augroup("PHPCSGroup", {clear = true})

            vim.g.nvim_phpcs_config_phpcs_path = 'phpcs'
            vim.g.nvim_phpcs_config_phpcbf_path = 'phpcbf'
            vim.g.nvim_phpcs_config_phpcs_standard = 'WordPress' -- or path to your ruleset phpcs.xml

            -- Setup auto formatting for php files using phpcs
            vim.api.nvim_create_autocmd({"BufWritePost", "BufReadPost", "InsertLeave"}, {
                group = "PHPCSGroup",
                pattern = "*.php",
                command = "lua require'phpcs'.cs()",
            })
            vim.api.nvim_create_autocmd("BufWritePost", {
                group = "PHPCSGroup",
                pattern = "*.php",
                command = "lua require'phpcs'.cbf()",
            })

            -- vim.keymap.set("n", "<leader>lp", function() require("phpcs").cbf() end, {
            --     silent = true,
            --     noremap = true,
            --     desc = "PHP Fix"
            -- })
        end
    },
}

The project has a phpcs.xml file that looks like this:

<?xml version="1.0"?>
<ruleset name="10up PHPCS">
    <description>10up PHPCS extended.</description>

    <!-- Scan these directories -->
    <file>./themes/*</file>
    <file>./mu-plugins/*</file>

    <!-- Don't scan these directories -->
    <exclude-pattern>node_modules/</exclude-pattern>
    <exclude-pattern>vendor/</exclude-pattern>
    <exclude-pattern>dist/</exclude-pattern>

    <!-- Exclude the specific plugins we don't want to scan -->
    <exclude-pattern>./plugins/debug-bar/</exclude-pattern>
    <exclude-pattern>./plugins/debug-bar-slow-actions/</exclude-pattern>
    <exclude-pattern>./plugins/query-monitor/</exclude-pattern>

    <!-- Use the 10up rulset -->
    <rule ref="10up-Default" />

    <!-- Ignore filecomment for the plugin loader -->
    <rule ref="Squiz.Commenting.FileComment.Missing">
        <exclude-pattern>./mu-plugins/animations-plugin-loader.php</exclude-pattern>
    </rule>

    <arg value="sp"/> <!-- Show sniff and progress -->
    <arg name="colors"/> <!-- Show results with colors. Disable if working on Windows -->
    <arg name="basepath" value="."/> <!-- Strip the file paths down to the relevant bit -->
    <arg name="extensions" value="php"/> <!-- Limit to PHP -->
</ruleset>

However one other problem I found is that when I save, my formatter isnot the defined in the file 10up-Default but WordPress instead which I have globally in phpcs and in the plugin config. I read that using the phpcs.xml file is a TODO, so I don't expect this to work., but the other error is really annoying.

However, when I open the project in a deeper folder, the error doesn't show up. It does only when a phpcs.xml is at the level I run nvim from.

praem90 commented 3 months ago

Thanks for reporting this issue and the detailed description, @xavortm ! I appreciate you letting me know about this.

I see it was brought to my attention previously by @mokorana #8 .

I'm happy to inform you that I've addressed this issue by adding a setup method to configure the options. This should allow for more control over the default config.

require("praem90/nvim-phpcsf").setup({
  phpcs = "phpcs",
  phpcbf = "phpcbf",
  standard = "10up-Default"
})

If you have any further questions or encounter any issues with the new setup method, please don't hesitate to let me know.

xavortm commented 3 months ago

Thank you for the quick update! I still can't get it to work though :( I have updated the plugin, so from Lazy I can see it showing the latest commit, so it should be pulling your latest update, then in my config I have added it as you said:

return {
    {
        "praem90/nvim-phpcsf",
        config = function()
            require("praem90/nvim-phpcsf").setup({
                phpcs = "phpcs",
                phpcbf = "phpcbf",
                standard = "10up-Default"
            })

            vim.api.nvim_create_augroup("PHPCSGroup", {clear = true})

            -- vim.g.nvim_phpcs_config_phpcs_path = 'phpcs'
            -- vim.g.nvim_phpcs_config_phpcbf_path = 'phpcbf'
            -- vim.g.nvim_phpcs_config_phpcs_standard = 'WordPress' -- or path to your ruleset phpcs.xml

            -- Setup auto formatting for php files using phpcs

...

(I also tried without the praem90/ prefix.

But when I try to open nvim now I get this error:

Screenshot 2024-06-13 at 07 53 21@2x

I am still quite new to vim in general, It could be an obvious mistake, but I did compare to other configs I have in Lazy and I think it's following the same structure

Viewing the plug in from Lazy I see this: Screenshot 2024-06-13 at 07 55 25@2x The commit seems the same as https://github.com/praem90/nvim-phpcsf/commit/87d34cdbfff69dc1744313415aa0f96fbf92758f

praem90 commented 3 months ago

Try this

require("phpcs").setup({
  phpcs = "phpcs",
  phpcbf = "phpcbf",
  standard = "10up-Default"
})
xavortm commented 3 months ago

No error now in opening neovim, but now when I open PHP files, even if there is no xml file in the root I get this: Screenshot 2024-06-13 at 08 04 39@2x

For reference, this is the lua file that is errored in: https://gist.github.com/xavortm/135c58c3326be8221711e3513ea39b3f in case it's of help

praem90 commented 3 months ago

It looks like the stdout of the phpcs is not a valid json.

praem90 commented 3 months ago

Try to load the branch debug and share with me the response printed.

xavortm commented 3 months ago

Okay; but just to be sure - when I do not add the require() from above, it works, and I can format, have the inline error messages and all. The only downside is that it was not using the 10up-defaults standard. Does that mean that the phpcs json output changes when I use require or there is something else going on?

xavortm commented 3 months ago

While on debug, the error when opening a PHP file is different now: Screenshot 2024-06-13 at 08 32 51@2x

praem90 commented 3 months ago

Its a typo. Could you please update it again?

xavortm commented 3 months ago

Sure, there it is: Screenshot 2024-06-13 at 08 39 35@2x

Does that error mean my standard is not globally available? Or rather, does it requrie it to be globally available or it uses what's in the xml I shared at the top?

When I run phpcs -i I get this:

The installed coding standards are MySource, PEAR, PSR1, PSR2, PSR12, Squiz, Zend, Modernize, NormalizedArrays, Universal, PHPCSUtils, WordPress, WordPress-Core, WordPress-Docs and WordPress-Extra

So I can see the standard is not available, but I thought it would read it from the local project. Or it doesn't work like that?

xavortm commented 3 months ago

*Edit - in my composer.json I do have the package "10up/phpcs-composer": "^3.0", (git repo - https://github.com/10up/phpcs-composer/tree/master) which should include the xml file I think. I did re-try to install composer packages (install or update) and the error is the same

I am testing a little more as I have some missing tokens and will write back

praem90 commented 3 months ago

Yes, if you do not specify any standard it will detect the phpcs.xml file in the root folder.

If you want to specify the standard name then it should be available in phpcs's list of standard.

So, in your case you dont need to call the setup function. The plugin must detect the phpcs.xml ruleset

xavortm commented 3 months ago

Thanks :) I have made two changes:

Removed the setup and changed to vim.g.nvim_phpcs_config_phpcs_standard = '10up-Default' -- or path to your ruleset phpcs.xml where before it said 'WordPress'.

In a php file now I get this error: { "\27[31mE\27[0m 1 / 1 (100%)", "", "", '{"totals":{"...able":false,"type":"ERROR","line":1,"column":1}]}}}' }

Or should I not change the global standard value? In 100% of my projects I do use this standard, but I can't get it installed globally so I was hoping it would read it from the repositories I work in as they all have it from composer.

praem90 commented 3 months ago

Yes, if you add this 10up/phpcs-composer repo as a dependancy in your project's composer.json it might work.

Also, setting this vim.g.nvim_phpcs_config_phpcs_standard = '10up-Default' will be overridden if a phpcs.xml file is exists in your root directory.

praem90 commented 3 months ago

Could you please share me the stdout of the below command from your project's root folder

./vendor/bin/phpcs --report=json .

xavortm commented 3 months ago

Reverted to use 'WordPress' then 👍

Output from the command from root: https://gist.github.com/xavortm/2609bffdebaa803ef52d420c8221c381

My composer.json does include it as a dep, all projects I have tested on do:

...
"require-dev": {
    "10up/phpcs-composer": "^3.0",
...
praem90 commented 3 months ago

Could please confirm that you have provided the --report=json option ?

If yes then it should print the json string

xavortm commented 3 months ago

Apologies, I didn't knew of the command; here it is -> https://gist.github.com/xavortm/69cc30f81c818674c9488398bc817e10

I have stripped the first line with the ....ww...ww content and formatted the json for easy reading.

praem90 commented 3 months ago

No worries. Actually those dots and progress should not be shown.

Let me fix this real quick.

Thanks again for your patience and for bringing this to my attention! I appreciate you helping me debug the issue.

praem90 commented 3 months ago

Great! I've fixed the issues you reported. Please let me know if you have a chance to test it and encounter any further problems.

xavortm commented 3 months ago

This is amazing, it works perfectly now! Of course, I still have to open my vim instance from the place where the xml file is, but that is totally fine I think. Thank you for the time and work on this, really really appreciated!

praem90 commented 3 months ago

I'm happy to report that the fix is in place and should resolve the issue you reported. Please let me know if you encounter any further problems.