search

spaze / phpstan-disallowed-calls

PHPStan rules to detect disallowed method & function calls, constant, namespace, attribute & superglobal usages
MIT License
255 stars 17 forks source link

Supporting attributes restrictions on more targets, i.e. properties #224

Closed francescolaffi closed 10 months ago

francescolaffi commented 10 months ago

Currently disallowedAttributes support only on attributes that target classes/interfaces/... or functions/methods (by AttributeUsages::processNode)

But attributes can have other targets, I discovered this limitation while trying to restrict an attributes that is used on class properties.

I think it should be possible for AttributeUsages to support nodes for the other targets, but I'd like to know where @spaze stands on this:

  1. lets add support for properties only, and see if someone else needs other targets in the future
  2. lets support all possible attribute targets, by detecting the specific php parser nodes
  3. lets support all possible attribute targets, by duck typing on attrGroups
  4. not adding support for other attribute targets

I'd go for 2, let me know what you thing and if you'd welcome a PR (probably in a few weeks) or if you'd like to handle it directly

spaze commented 10 months ago

4 is also fine 😂 But really, I haven't added support for other targets because it was more work at the time and the extension doesn't support properties elsewhere (which I admit is more of an excuse). But I thought I had at least documented it which turned out that I haven't, sorry!

I'm fine with adding support for just properties plus document the limitation, or add full support, and then probably no documentation changes needed. Not sure what's easier now 😅

Duck typing doesn't sound like something I'd like to see but may be needed, I don't know right now. PR is always welcome!

spaze commented 10 months ago

Hey @francescolaffi I got somehow interested more now (= can't sleep) 😄 and created #225. It's marked as Draft because there are more nodes (in PhpParser teminology) that can be targeted with an atribute like enums, and possibly more, that are not yet supported. You can work on that PR if you want/can, otherwise I'll probably add those in the upcoming days.

spaze commented 10 months ago

All targets are now supported, would still like to add tests for attributes for arrow function, closure, function, interface, trait.

francescolaffi commented 10 months ago

thanks @spaze! that was fast :)

tested it out from the main branch and it works as intended, notices something minor addressed in #229