[Call-by-name] Replaced ast with libcst in migration code

[sureshjoshi]

This PR addresses several ast-centric implementation problems. Using a concrete syntax tree allows us to do fancier migrations, and not have to write code to handle silly things like trailing commas or nested comments.

High level, libcst is amazing. All of the most complex code in the migration was replaced by a better transformer.

• Gets with comments within parameter/args are now permitted (we might run into hiccups, but the hiccups are now related to implementation - not the entire concept of using an ast)
• Having multiple Gets on the same line in a MultiGet no longer wipes out the latter Gets (Closes [Call-by-name]: Migration does not handle multiple MultiGet in the same line #21066)
• Started on smarter implicitly usage (closes [Call-by-name] Handle implicitly cases better #21072) - for example, functions that take no arguments no longer have an implicitly attached
• Gets within rule_helpers (or, now, just non-rule-decorated async functions) CAN BE correctly transformed (right now, the rule engine itself doesn't seem to pick these up)

The implicitly work done in this PR will eventually get pulled out into a custom linter/type-checker, as outside of this migration - we'll want to flag when users add new code with unnecessary implicitly usage.

Closes #21040

[sureshjoshi]

In subsequent PRs (or, as part of subsequent migrations), I'll add some more implicitly rules and clean up some of the code. This is a visually a big change, that I didn't want to get much bigger.

Overall, less code, more functionality

[cognifloyd]

I read through it, but my understanding of the call by name conversion is superficial, so someone else needs to review this in more depth. From what I can tell, LGTM.

[sureshjoshi]

I read through it, but my understanding of the call by name conversion is superficial, so someone else needs to review this in more depth. From what I can tell, LGTM.

FWIW the end result should be "same as before, except with some bugs fixed". Most of the re-factoring was what was required to switch from AST to libcst. The only "novelty" was one implicitly optimization (removed when the called function requires args)

[sureshjoshi]

Not needed, handled by libcst transformer

[sureshjoshi]

Not needed, handled by libcst transformer

[sureshjoshi]

Not needed, handled by libcst. ast wipes out comments by default, cst keeps them

[kaos]

do we want to guard this from leaking into pants runtime (beyond the migration code)?

pants/3rdparty/python/BUILD

Lines 31 to 44 in 777c282

	__dependents_rules__(
	( # Only the explorer server may depend on these libraries
	(
	"[fastapi]",
	"[starlette]",
	"[strawberry-graphql]",
	"[uvicorn]",
	),
	"pants-plugins/pants_explorer/server/**",
	"!*",
	),
	# Free for all on the rest
	("*", "*"),
	)

[cognifloyd]

I think libcst is an excellent utility for migrations, of which this will not be the last. Allowing broader use of it seems like a plus to me. So I don't think we should restrict this dependency with dependency visibility rules.

[kaos]

I was feeling this could be useful as well, raised the question to get a conscious decision for it though, as there's been an on-going effort to reduce the number of 3rd party dependencies (albeit controversial at times) in #19282, so it would be good to have a clear record on the motivation for each dependency and it's intended use, I think.

[sureshjoshi]

Pulling these from source comments to main:

do we want to guard this from leaking into pants runtime (beyond the migration code)?

I think libcst is an excellent utility for migrations, of which this will not be the last. Allowing broader use of it seems like a plus to me. So I don't think we should restrict this dependency with dependency visibility rules.

I agree :)

so it would be good to have a clear record on the motivation for each dependency and it's intended use

I also agree :)

The motivation here is that libcst will be used in some implicitly checks and in the upcoming migrate goal I'm writing (or migrate fix-er`), so it'll end up in source code regardless.

I agree, overall, that we would like to reduce dependencies - or visibility them out.

I wonder if pex's new PEP723 support (or __visibility__) will help for those places where we have one-off build files, if they're not already hidden away from the rest of the code.