pdt icon indicating copy to clipboard operation
pdt copied to clipboard

Missing validation on non lowercase keyword

Open the-liquid-metal opened this issue 1 year ago • 9 comments

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

  • PHP Version: 8.2.7, 8.1.20, 8.0.29, 7.4.33 (via https://onlinephp.io/)

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);
        }
    }
}

the-liquid-metal avatar Jun 18 '23 05:06 the-liquid-metal

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).

mlocati avatar Jun 18 '23 06:06 mlocati

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 avatar Jun 18 '23 20:06 the-liquid-metal

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

the-liquid-metal avatar Jun 18 '23 20:06 the-liquid-metal

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

zulus avatar Jun 21 '23 10:06 zulus

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).

mlocati avatar Jun 21 '23 15:06 mlocati

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

zulus avatar Jul 04 '23 12:07 zulus

With pleasure.

the-liquid-metal avatar Jul 04 '23 17:07 the-liquid-metal

@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 avatar Jul 07 '23 20:07 the-liquid-metal

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.

the-liquid-metal avatar Jul 07 '23 23:07 the-liquid-metal