eclipse-pdt / pdt

PHP Development Tools project (PDT)
https://eclipse.org/pdt
Eclipse Public License 2.0
188 stars 46 forks source link

Missing validation on non lowercase keyword #235

Open the-liquid-metal opened 1 year ago

the-liquid-metal commented 1 year ago

Bug Description Missing validation on non lowercase keyword.

Eclipse environment Version: 2023-06 (4.28.0) Build id: 20230608-1333 PDT: 8.0.0.202306050832

System

To Reproduce Steps to reproduce the behavior: copy-paste this script to the IDE

<?php
declare(strict_types=1);

NameSpace ns1\ns2;            // this statement should trigger warning

use STDCLASS;                 // this statement should trigger warning

CLASS Test61 {                // this statement should trigger warning

    PUBLIC int $prop1;        // this statement should trigger warning

    public STATIC int $prop2; // this statement should trigger warning

    Function fn1(int $par1, BOOL $par2) { // this statement should trigger warning
        IF ($par2 == TRUE) {              // this statement should trigger warning
            ECHO "OK";                    // this statement should trigger warning
            $var1 = new STDCLASS;
            var_dump($var1);
        }
    }
}
mlocati commented 1 year ago

PHP doesn't care about upper/lower/mixed case of the keywords (for example, iNt is absolutely legal). This is just your personal preference, and you can use tools like php-cs-fixer to fix/check your code style accordingly.

And please, don't spam this repo: I'm not sure that the project maintainer will have time to even read the ton of issues from you (he's maintaining PDT for free in his spare time, so please respect his time).

the-liquid-metal commented 1 year ago

I apologize if you feel I have spammed this repo :pray:. I do not mean it like that. On the contrary, I wrote it because I appreciate the work done by the maintainer. The PDT is a good quality tool, and deserves further improvement. At this time I have not been able to contribute a patch that fixes those errors. The only way is to report all the errors I find. If the maintainer uses his free time, then my form of appreciation is to use the free time this weekend.

I found all these errors all day long. This is better for me because I can focus, instead of putting a little time in each week. And the result is a fairly comprehensive list of issues, which I probably wouldn't have gotten around to if time had been fragmented. By just reading the title, you can predict the content. If you read the contents, you will find case examples that are also comprehensive. Luckily, all the issues I wrote about were not interrupted by other people's issues, so that a kind of solid narrative was formed.

I'm aware that my findings are flooding this repo. I report only the most basic issues. The more errors I find now, the harder it will be for me to get them again later. That's why I do something like this only once (or maybe twice) in my lifetime. We all know that this fix takes a long time, and I absolutely did not expect it to be finished in two or three releases. The maintainer can choose the issues he thinks are worth working on now, and postpone the others at a later time. Please don't feel burdened.

For case problems on keywords (and several other things that haven't been made an issue of yet), my goal in proposing a new validation is to train habits/form the user's mentality to adhere to consensus, and not just rely on other tools. Some people trivialize the issue of consensus and leave the repair work to colleagues. Even though the use of other tools is easy enough, this sort of thing sometimes still creates friction. If an error is raised, at least there's less reason to be evasive for people who don't comply. So, this kind of issue is not merely a personal choice. But this is all left up to the maintainer, whether this proposal will be accepted or not.

Once again I apologize if what I did gave a negative impression :pray:.

the-liquid-metal commented 1 year ago

Almost forgot, thanks a lot for this great tool. Even though I use other applications at work, I still enjoy using PDT.

zulus commented 1 year ago

This is feature request to introduce more validation for code style.

I have also plan to include more php tools integrations, in this case good cs fixer integration will resolve this (and probably other ) tasks, but still good „code cleanups” framework similar to JDT will be faster

mlocati commented 1 year ago

I agree that we should use already existing tools whenever possible.

About coding style (the subject of this issue) I'd use PHP-CS-Fixer: it's well maintained and full of features (see for example this page of mine, where you can see a list all the configurable coding styles).

zulus commented 1 year ago

@the-liquid-metal could you collect this tasks as one single ticket with table/todo list? It's hard to navigate right now

the-liquid-metal commented 1 year ago

With pleasure.

the-liquid-metal commented 1 year ago

@the-liquid-metal could you collect this tasks as one single ticket with table/todo list? It's hard to navigate right now

Done. Please see #245

Feel free to copy and change the state, since you are not able to change mine.

the-liquid-metal commented 1 year ago

Feel free to copy and change the state, since you are not able to change mine.

On second thought, we should just use the same thread and use comments. You can still update the comment at any time. I've included the original text for copy-pasting in case you haven't noticed.