search

phpactor / language-server-phpactor-extensions

MIT License
0 stars 4 forks source link

Generate method #40

Closed dantleech closed 3 years ago

dantleech commented 3 years ago

This PR updates the method generator to use the ne MissingMethodFinder and act on the entire document:

@BladeMF note that as we no longer use the parser, the tests have been simplified, the deleted test cases could be moved to code-transform however.

I also notice that, at least for me, generating methods in a foreign class does not place the method in the correct place (probably the source or offset is being wrongly mapped).

dantleech commented 3 years ago

I've added the additional test cases to code-transform

BladeMF commented 3 years ago

I have become really unsure of having diagnostics enabled. I mean, phpstan will tell me about undefined methods anyway, I really don't want blue or yellow or red squiggly lines under methods that phpactor can't resolve. I still support showing the action only on unresolvable methods though.

What I suggest is at least make it an option to turn those off. I, as a developer, am really bugged by squggly lines - phpstan's underscores on MethodProphecy methods drive me NUTS. What do you think?

dantleech commented 3 years ago

What I suggest is at least make it an option to turn those off

You can turn them off: https://phpactor.readthedocs.io/en/master/reference/configuration.html#language-server-diagnostics-on-update but I guess you mean more granularly?

BladeMF commented 3 years ago

Yes, just these diagnostics - for generate method. Do you have diagnostics where you work? VIM? Having all unknown methods underlined is basically what phpstan does after a certain level.

BladeMF commented 3 years ago

I guess I can try it and see.

My whole point is that this can be annoying if there are a lot of objects whose types are not inferred correctly (or, all prophecies, for that matter) - your whole file could light up with no way to turn it off, but those would not be generatable methods (or those would not light up?).

I think just having the light bulb (code actions) showing up after you step on an unknown method will be enough. This is how Visual Studio (not Code) does all refactorings - it offers them after you've selected the code or stepped on the property or method. So nobody would expect phpactor to highlight all possible generate method sites in the file in advance (which is diagnostics).

dantleech commented 3 years ago

Yes we can proably remove the diagnostics or make them optional :)

BladeMF commented 3 years ago

Prophecised methods get underscored :-( I actually don't mind the diagnostics elsewhere for now, but here it is annoying. image

  1. Should that be solved with generics?
  2. Probably the same mechanism that will give us the completion of the methods for ObjectProphecy objects?
  3. Should we try to turn of diagnostics for those kind of objects for now?
dantleech commented 3 years ago

I don't mind adding some mechanism to specific diagnostics. Generics AND an extension would solve this, but generics are still a long way away (I would need weeks of dedicated time probably)

BladeMF commented 3 years ago

Not sure what to do either. Probably a "turn off diagnostics for undefined methods" setting would be the easiest solution.