openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.29k stars 340 forks source link

yaml.DeleteProperty create a malformed yaml #1125

Closed ruben-garciapariente closed 3 years ago

ruben-garciapariente commented 3 years ago

Hi

Thanks for this wonderful proyect :)

I did another test with org.openrewrite.yaml.DeleteProperty and rewrite-maven-plugin:4.13.0

My recepy is

---
type: specs.openrewrite.org/v1beta/recipe
name: com.yourorg.AddDependencyExample
displayName: Add Maven dependency example
recipeList:
  - org.openrewrite.yaml.DeleteProperty:
      propertyKey: com.company.other.name
      coalesce: true

Source yaml is

com:
  company:
    other:
      name: project-test
      logging:
        kafka:
          uniqueTopic: a-topic
#TODO bug pending          topic: a-topic #This property disappear
          properties:
            one.other: true

I did mvn rewrite:run

[INFO] --- rewrite-maven-plugin:4.13.0:run (default-cli) @ test-openrewrite ---
[INFO] Using active recipe(s) [com.yourorg.AddDependencyExample]
[INFO] Using active styles(s) []
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by nonapi.io.github.classgraph.utils.ReflectionUtils$StandardReflectionDriver$1 (file:/C:/Users/n129586/.m2/repository/io/github/classgraph/classgraph/4.8.123/classgraph-4.8.123.jar) to method sun.management.RuntimeImpl.getInputArguments()
WARNING: Please consider reporting this to the maintainers of nonapi.io.github.classgraph.utils.ReflectionUtils$StandardReflectionDriver$1
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
[INFO] Validating active recipes...
[INFO] Parsing Java main files...
[INFO] Parsing Java test files...
[INFO] Parsing YAML files...
[INFO] Skipping properties files because there are no active properties recipes.
[INFO] Skipping XML files because there are no active XML recipes.
[INFO] Skipping Maven POM files because there are no active Maven recipes.
[INFO] Running recipe(s)...
[WARNING] Changes have been made to src\main\resources\config\application.yml by:
[WARNING]     org.openrewrite.yaml.DeleteProperty
[WARNING] Please review and commit the results.

then I get a malformed yaml

com.company.other.logging.kafka:
  uniqueTopic: a-topic
g pending          topic: a-topic #This property disappear
          properties.one.other: true

The complete example is in https://github.com/ruben-garciapariente/test-openwrite/tree/62825927e1a11268dfdb1f261cb9ac12dc56858e

Thanks & regards

aegershman commented 3 years ago

Hey @ruben-garciapariente! Good stuff, thanks for logging this, we'll take a look 👍

aegershman commented 3 years ago

Also, @ruben-garciapariente -- for the time being, if you set coalesce: false, you should not get the malformed yaml document. It won't be formatted the same way, but it won't break the yaml document. Hopefully that prevents you or anyone from getting fully stuck

aegershman commented 3 years ago

Hey @ruben-garciapariente -- after having merged this PR (https://github.com/openrewrite/rewrite/pull/1132), I tried out the example you provided in https://github.com/ruben-garciapariente/test-openwrite/blob/62825927e1a11268dfdb1f261cb9ac12dc56858e/src/main/resources/config/application.yml and the result looks more appropriate now:

Before:

com:
  company:
    other:
      name: project-test
      logging:
        kafka:
          uniqueTopic: a-topic
#TODO bug pending          topic: a-topic #This property disappear
          properties:
            one.other: true

After:

com.company.other.logging.kafka:
  uniqueTopic: a-topic
#TODO bug pending          topic: a-topic #This property disappear
  properties.one.other: true

This ought to be things taken care of for you. If you try out the latest changes and things aren't to what you're expecting, definitely reach out again and let us know. Thanks again for logging this, it helps 👍