pixee / codemodder-python

Python implementation of the Codemodder framework
GNU Affero General Public License v3.0
32 stars 10 forks source link

Suggestion: Add warning messages in LibcstTransformerPipeline #522

Open stephenpaulger opened 2 months ago

stephenpaulger commented 2 months ago

The code where I think it might be worth adding a warning is in the apply method of LibcstTransformerPipeline.

        try:
            with file_context.timer.measure("transform"):
                for transformer in self.transformers:
                    tree = transformer.transform(tree, results, file_context)
        except Exception:
            file_context.add_failure(file_path)
            logger.exception("error transforming file %s", file_path)
            return None

        if not file_context.codemod_changes:
            return None

        if not (diff := create_diff_from_tree(source_tree, tree)):
            return None

In the case where tree has changes but the codemod author has not added to the file_context.codemod_changes array the changes they are expecting to be made will be discarded. I would say that when both conditions are False it's likely the codemod author will expect nothing to happen but in cases where only one is False it would be useful for there to be a warning printed as I think any time there is a element in file_context.codemod_changes or there is a tree diff the codemod author intended for something to happen.

clavedeluna commented 2 months ago

Hi @stephenpaulger ! Thanks for the suggestion! If I understand it correctly, I think unit tests would fail which should warn a codemod author that something is up. But we're open to PRs if you'd like to contribute.

stephenpaulger commented 2 months ago

It's possibly something that will be less important once there's some more documentation. While I was trying to create a codemod starting from the cookiecutter template that was working with just libcst I was seeing that my code to replace a node was running but the changes weren't applied and I had to use pdb to go up the stack to find why.

I'm happy to send a PR given you're open to them 😃