DEVSENSE / phptools-docs

PHP Tools public content
Apache License 2.0
77 stars 10 forks source link

Show error when class name does not match the containing file name #609

Open jrmajor opened 1 month ago

jrmajor commented 1 month ago

Sometimes I rename a class but forget to rename the file. The editor does not warn me about it, but it results in a runtime error, since the class now cannot be autoloaded correctly. PhpStorm has such warning and I miss it after switching to VS Code.

Edit: The error I've described above is for classname–filename discrepancy. Directory–namespace discrepancy should also be validated. In such cases, PhpStorm shows a „namespace doesn't match the PSR-0/PSR-4 project structure” error.

Would also be nice if there were automatic fixes for that: “rename class”/“rename file” and “change namespace to match folder structure”/“move file to correct directory”.

jakubmisek commented 1 month ago

Good idea;

jrmajor commented 1 month ago

Any hint wow do we know, the project is PSR0/PSR4?

You could check autoload field in composer.json.

jakubmisek commented 1 month ago

You could check autoload field in composer.json.

Perfect, makes sense.

jakubmisek commented 1 month ago

How would you suggest handling classes, that are not supposed to be autoloader?

Note: other IDEs just report that.

For example, the user may have a helper file included manually (I know the file should be listed in "autoload"\"files", but it isn't in this case). Is it something people may find disrupting when we report that?

Example: composer.json:

{
  "autoload": {
    "psr-4": {
      "Vendor\\Package\\": "src/"
    }
  }
}

src/helpers/includes.php:

<?php
class Helper {
}

Now, we will report a warning, that class "Helper" should be either renamed to "includes" or the path should be changed.

jrmajor commented 1 month ago

After some time with v1.49.15843, I find the new inspection is super useful. I often create a new class by copying a similar file and editing it, so I've already used the fix many times.

I'd suggest that it should be an error (red squiggle instead of blue one). In most cases, it is a serious issue, and I don't think false positives would be as common as you think. In PhpStorm this is an error, and I don't remember seeing any false positives, even when working with legacy projects.

I also find quick fix names a bit confusing: screenshot-2024-08-10T15 18 24@2x First action doesn't say what it really does, and the second one has “File” with the uppercase first letter, making me think that it's the class name. I'd suggest renaming them to “Rename class 'Foo' to 'Bar'” and „Rename file 'Bar' to 'Foo'”, or if you want to make it shorter, “Rename class to 'Bar'” and „Rename file 'Foo'”. This is how it looks in PhpStorm and I find it much easier to understand: screenshot-2024-08-10T15 21 23@2x

I'd also love to see a corresponding inspection for namespaces.

How would you suggest handling classes, that are not supposed to be autoloader? For example, the user may have a helper file included manually (I know the file should be listed in "autoload""files", but it isn't in this case). Is it something people may find disrupting when we report that?

If you explicitly stated in composer.json that the directory should follow PSR-4, you probably want to see a warning when it doesn't. Composer itself will also yell at you when you run composer dump-autoload -o.

When working with legacy code, it's unavoidable that there will be some false positives, not only with this inspection, but with other ones too. I'd be OK with such tradeoff. It's something that you probably should fix, and it's an easy fix — in most cases you can add the file to autoload.files or move it to a directory that isn't following PSR-4.

jakubmisek commented 4 weeks ago

Good point! Code actions title will be adjusted, error severity as well.

We'll taker a look on the namespace quick fix.