cspell complains about phpcs sniff name

Created on 31 July 2024, 9 months ago

I have this line of code in one of my modules:
// phpcs:disable DrupalPractice.General.OptionsT.TforValue

But cspell complains about this, saying Unknown word (Tfor)

I believe this is because the sniff name does not follow proper camel casing. Maybe if it were named properly cspell wouldn't complain? Or at the very least, ALL sniff that we use ought to be in the dictionary so they aren't flagged.

What would be the proper fix here?

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
Other 

Last updated 1 day ago

Created by

🇺🇸United States tr Cascadia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @tr
  • 🇮🇳India abhiyanshu

    @TR I would suggest to update the cspell Dictionary and add specific terms to ignoreWords if they are valid in your context.

    Simply Update/add TforValue to ignoreWords in .cspell.json, save it, and run cspell again to check if it stops flagging TforValue.

    {
      "version": "0.1",
      "ignoreWords": [
        "TforValue"
      ]
    }
  • 🇺🇸United States tr Cascadia

    Yes, of course I can do that. I could also turn off cspell entirely.

    But it's silly that Drupal-specific phpcs sniff names are not automatically excluded.

    I mean, solving a phpcs issue with a phpcs:ignore causes a new cspell issue. That's just silly.

    Either cspell should ignore things like phpcs directives or the phpcs sniffs should use naming/casing conventions that are recognized by cspell.

    I *think* if it were named DrupalPractice.General.OptionsT.TForValue instead of DrupalPractice.General.OptionsT.TforValue then cspell wouldn't complain. (TFor instead of Tfor).

    And since incorrect camel casing is considered a phpcs error, it's again very silly that the phpcs sniffs have incorrect camel casing.

  • 🇳🇿New Zealand quietone

    This is not something that can be fixed in core. The sniffs are named by the Coder Module. I am moving this there.

    There is another sniff that also includes a spelling error, according to our use of cspell. That is Drupal.CSS.ColourDefinition. There is an issue in Coder asking for it to be renamed, 🐛 Rename ColourDefinitionSniff to ColorDefinitionSniff Needs work . I suggest you read comment #5 in that issue.

  • 🇦🇹Austria klausi 🇦🇹 Vienna

    Coder sniff names are internal error codes - sometimes we have to build camel case word combinations to make the error code useful.

    So we cannot fix the sniff names, because people rely on the codes now in their configuration. And we should not change error codes just to make cspell happy.

    So that leaves us with the option to exclude those words in cspell, where would be a good place for that? Probably Drupal core's dictionary file? Is is possible to put long dot works like "DrupalPractice.General.OptionsT.TforValue" there?

  • 🇬🇧United Kingdom jonathan1055

    I commented on #3418190-10: Rename ColourDefinitionSniff to ColorDefinitionSniff and the same solution can be done here. It is not practical to rename the sniff, nor is it practical to ask any contrib maintainer to add this word to their ignore list. We just need to add into the Core dictionary of words to ignore. See core/misc/cspell/dictionary.txt

  • 🇬🇧United Kingdom jonathan1055

    I have now moved 🐛 Rename ColourDefinitionSniff to ColorDefinitionSniff Needs work into the Core issue queue, to request that these words are added to the Drupal dictionary.

  • 🇦🇹Austria klausi 🇦🇹 Vienna

    Thanks, closing this as won't fix in Coder.

  • 🇺🇸United States tr Cascadia

    And now we've come full circle.

    I opened this issue in Core, where it was declared as not a core issue by @quietone and moved to Coder. See #4.

    Now it's not a Coder issue anymore and it's "won't fix".

    Note that my original post, back in July, did suggest two things:

    1. Add the sniff names to the core dictionary, or
    2. Correct the sniff names to follow core Drupal naming standards

    Both of these are still viable options. Core objects to 1) and Coder objects to 2). I don't see how moving this back to Core and saying core should do 1) will get us anywhere, because that was rejected back in July.

    And it feels like I'm being cut out of the conversation after 6 months and that my input so far is being thrown away, because this issue is now "won't fix" and the new issue does not preserve my contribution to this issue.

    It is this dysfunctional Drupal core issue handling that has left so many little things unresolved for so many years. It can take 10 years to get a small patch like this through the system. Not an exaggeration - I can cite many examples. While major BC-breaking changes to core can happen in just a month. Again, not an exaggeration.

    I find it enormously disrespectful and discouraging. This process has been a waste of my time and illustrates why community participation has dropped precipitously since the Drupal 7 days - core is sending the message that it doesn't care for community contributions.

    Thank you @jonathan1055 for trying, but the fundamental problem seems to be that core maintainers don't really care about problems like this which affect contrib on a daily basis, and are unwilling to spend any time on the small issues.

  • 🇦🇹Austria klausi 🇦🇹 Vienna

    @TR sorry for the frustration, I can certainly relate to the slow processes in open source projects.

    In this case it took some time until we figured out where changes should happen. Thank you for your contribution of the bug report and the discussion.

    However, you cannot expect the Drupal core maintainers or me as Coder maintainer do the work for you. We can give you direction and we can review merge requests when we have time, but the work is assigned to the community (you are part of that group).

    I find it disrespectful to blame core maintainers here. quietone has merely moved an issue around to figure out the best solution for the problem. Now we come to the conclusion that we rather want to fix this in core, so we moved it back. If you want to blame someone then you can blame me because this issue has been sitting in the Coder queue for a while. I'm happy to explain to you how I work as maintainer (basically ignoring everything that is not RTBC, except when I have time to take a deeper look). You are a community member for 17 years - come on, you know how this works.

    I also find it strange that you complain about a dysfunctional core issue queue when there has been so much improvement recently (check out the bug squash initiative).

    Lastly, I want to thank the core maintainers and quietone. I have seen all the coding standards contributions you have been pushing, great work! I'm very happy to work with you. Don't let insults in the issue queue drag you down, I fully support you!

  • 🇬🇧United Kingdom jonathan1055

    Hi TR,
    Ah, yes I totally get your frustration. I had not re-read the top of this thread when I resumed working on 🐛 Rename ColourDefinitionSniff to ColorDefinitionSniff Needs work . However, things have changed a little bit since @quietone posted in #4 above. They were saying that the sniff can't be fixed in core because the names are defined in Coder. But they were not making any comment on the dictionary solution, just passing it to the Coder queue. We did have some discussion on whether the sniffs could be renamed but it seemed not possible, and the only sensible and practical (and extremely easy) solution is to add the name to the core dictionary.

    I am sure @klausi did not intend to cut you out of the conversation, it just seemed that way when the issue was changed to 'closed (wont fix)'. I know there won't be a change in Coder for this, but I've re-opened the issue, as it has lots of good history and should remain visible in the queue, to remind us that we have the other one in the core queue.

    If the proper core solution does not materialise, there is another way, because I am also involved in many of the Gitlab Templates support issues (and co-incidentally I was responsible for adding the CSpell job into the contrib pipelines). There is an open issue 📌 Add new dictionary to cater for common words not in core dictionaries Active where for the sake of sanity of maintainers we added some common words that had been dropped from the core dictionary between Drupal 11.0 and 11.1. That list can very easily be expanded to cover the six strings we are talking about in these issues.

    So if we don't get any response from the core solution, I may move the issue again, into the Gitlab Templates queue :-)

    Jonathan

  • 🇬🇧United Kingdom jonathan1055

    Re-opening as that change I made was lost. Hope that's OK @klausi. The issue can be closed again when resolved.

  • 🇺🇸United States tr Cascadia

    However, you cannot expect the Drupal core maintainers or me as Coder maintainer do the work for you. We can give you direction and we can review merge requests when we have time, but the work is assigned to the community (you are part of that group).

    I think it's very clear from my contribution history that I am perfectly willing to do the work. That has never been an issue. And you know that. My entire tenure of involvement in Drupal has been doing work (on a volunteer basis) that no one else is willing to do.

    While I have demonstrated that I am quite willing to *spend* time contributing, I am *not* willing to *waste* time doing work that has no chance to be accepted.

    Thus, when opening an issue I first point out a problem and posit a solution or two, and ask like I did above "What would be the proper fix here?". I expect the maintainers (core or Coder) to choose the way they want to resolve the issue.

    IF there is agreement as to what should be done, THEN I will do the work if I can. It would be a huge waste of my time to do work that no one wants.

    In this particular case, I personally would have solved it by renaming the Coder sniffs (with the necessary deprecations and testing) as well as ensuring through testing that no new sniffs get committed unless their names comply with Drupal coding standards. And like you say, it would be a lot of work and not the easiest solution. If I had done that work pre-emptively it would have been wasted effort because that course of action was rejected in #5.

    I am sure @klausi did not intend to cut you out of the conversation, it just seemed that way when the issue was changed to 'closed (wont fix)'.

    I was not trying to imply intent. I was just saying that it feels disrespectful when this happens. And it happens a LOT. At least a dozen time last year there were issues that I had been contributing to for YEARS, including writing patches and tests and re-rolls, that were closed because the issue was fixed by a new issue opened years later. A new issue where the poster had not even searched the issue queue for existing issues but the maintainer immediately accepted the solution that was offered in the newer issue - usually the same patch that I had working years before. And that's exactly why I will no longer offer complex patches up-front, until there is consensus for what should be done. It is IMO part of the maintainers role to set and/or approve direction and delegate implementation to those willing to take on the tasks, not just to sit in judgement. The maintainer has the birds-eye view, and is the one who should be saying "no, we're not going to go that way", or "yes, that looks like a good way to handle things so why don't you code that up and I'll commit it"

    Isn't that how things work in your day job? What you work on is discussed and agreed with your manager and/or employees and/or clients (whether that work is good enough is a different discussion...). You should never be in the position of spending months or years on a task that management doesn't want or clients won't pay for. If that's SOP in your company, then you're going to lose employees. Drupal is no different, and Drupal *has* lost contributors hand over fist over the past 10 years.

  • Status changed to Closed: won't fix about 1 month ago
  • 🇦🇹Austria klausi 🇦🇹 Vienna

    Thanks a lot Jonathan for fixing this in 📌 Add new dictionary to cater for common words not in core dictionaries Active .

    I don't think there is anything left to do in Coder, so will close this again.

  • 🇬🇧United Kingdom jonathan1055

    You're welcome. It seemed the best way to do it, and I wanted to get the situation resolved :-)

Production build 0.71.5 2024