call-by-name: I'm looking at migrating some encapsulated rules (

@rule

funcs that are in the body of a rule builder method) in the adhoc backend the the

migrate-call-by-name

script couldn't migrate. Which of these styles is preferable: Option 1: Expand the rules

cqt = await find_code_quality_tool(CodeQualityToolAddressString(address=self.target))
            code_quality_tool_runner_request = await runner_request_for_code_quality_tool(cqt)
            code_quality_tool_runner = await create_tool_runner(code_quality_tool_runner_request)

Option 2: Use implicitly

code_quality_tool_runner = await create_tool_runner(
                **implicitly(CodeQualityToolAddressString(address=self.target))
            )

find_code_quality_tool

and

runner_request_for_code_quality_tool

are rules in the same file.

create_tool_runner

is imported. This is what the same lines look like before migrating them:

code_quality_tool_runner = await Get(
                ToolRunner, CodeQualityToolAddressString(address=self.target)
            )

The migration tool jumps straight to implicitly, and it's a bit visually cleaner at the expense of some amount of runtime lookups. We haven't quantified the effect at runtime though

OK. I will push a PR with the calls expanded, and I can switch to

**implicitly(...)

if anyone cares.

If you're doing mass migrations, implicitly is easier/faster for the dev - which has value

Yeah, this one is a manual migration, so 🤷

If it helps you, I was already looking into the code_quality_tool stuff: https://github.com/pantsbuild/pants/tree/migrate-call-by-name/backend-adhoc

doh https://github.com/pantsbuild/pants/pull/22354

I gave up because running

./pants test src/python/pants/backend/adhoc/code_quality_tool_integration_test.py

resulted in lots of rule graph errors.

I think your issue was trying to use

execute_process(**implicitly(CodeQualityToolBatch

instead of just using the

process_files

rule that goes directly from

CodeQualityToolBatch

to

FallibleProcessResult

. At least, that's the only significant difference I see.

Oh wait, I think I tried to migrate that at some point as well

K. It looks like my PR has all tests passing now. 🙂