Allow usage of additional file extensions per parser.

[ammachado]

What's changed?

Allow parsers to parse additional file extensions.

What's your motivation?

Parse Karaf's configuration files as properties.

Anything in particular you'd like reviewers to focus on?

No

Anyone you would like to review specifically?

No

Have you considered any alternatives or workarounds?

No

Any additional context

No

Checklist

• [x] I've added unit tests to cover both positive and negative cases
• [x] I've read and applied the recipe conventions and best practices
• [x] I've used the IntelliJ IDEA auto-formatter on affected files

[knutwannheden]

I like the idea! But I wonder if the execution context shouldn't instead allow registering alternative implementations for accept(), so that file extensions can also be removed (let's say someone doesn't want to parse Jenkinsfile files anymore). This would require some further refactoring, so that an alternative implementation can still optionally delegate to the default implementation. Thoughts on this?

[ammachado]

Perhaps if we just add a file exclusion pattern, similar to the additionalExtensions implemented here, would work? ... accept(..) would check for the default extensions, then inclusions and exclusions.

[sambsnyd]

It isn't clear to me that we need to make this extensible. We're happy to add to the list of extensions for each parser. Do you have a file extension you'd like to accrue to one of the parsers by default? Do you have a scenario where hard-coding is insufficient or otherwise impossible?

[ammachado]

This PR adds the option to include/exclude file patterns per parser. In my original use case, I have properties files that have .cfg extension instead of .properties, and I want to use the existing property recipes to refactor them.

[jimschubert]

I also described my use case in #4062. I have a yaml file without extension. To parse this as YAML required me to not only subclass YamlParser and a few other types, but I had to pass the file pattern as plain text matchers to the Maven plugin. That functionally has been deprecated, so it's not clear to me how I'd go about targeting my desired file mappings without this PR.

[sambsnyd]

I would rather add an entry to the hardcoded list than add an entire extension mechanism. If all you need is to parse Karaf's configuration files as properties we can add it to the list of extensions that parser accepts by default.

[jimschubert]

Hard coding my company's internal tooling names into OpenRewrite isn't an acceptable solution.

[timtebeek]

@jimschubert Please note the plaintextMasks option is not deprecated; we only deprecated a duplicate option without a qualifying prefix.

To offer an alternative that's already possible using existing mechanisms, one could try a recipe that runs first to convert any PlainText parsed files into Yaml with a recipe; this is something that's come up on conversation before internally.

Show recipe that converts a plain text parsed file into Yaml

Note that I haven't checked the below code; just meant to illustrate the general approach.

package org.openrewrite.yaml;

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.openrewrite.*;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.text.PlainText;
import org.openrewrite.text.PlainTextVisitor;

import java.util.Optional;

@Value
@EqualsAndHashCode(callSuper = false)
public class AsYaml extends Recipe {

    @Option(displayName = "File pattern",
            description = "Ant-style path pattern matching source files to parse as YAML.",
            example = "com/myapp/**/*.acme")
    String filePattern;

    @Override
    public String getDisplayName() {
        return "Parse as YAML";
    }

    @Override
    public String getDescription() {
        return "Parse an existing PlainText file as YAML. Useful for when you have a file that is not recognized as YAML, but you know it is.";
    }

    @Override
    public TreeVisitor<?, ExecutionContext> getVisitor() {
        PlainTextVisitor<ExecutionContext> plainTextVisitor = new PlainTextVisitor<ExecutionContext>() {
            @Override
            public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
                if (tree instanceof PlainText) {
                    PlainText pt = (PlainText) tree;
                    Optional<SourceFile> first = YamlParser.builder().build()
                            .parse(tree.print(getCursor()))
                            .findFirst();
                    if (first.isPresent()) {
                        SourceFile sourceFile = first.get();
                        if (pt.getCharset() != null) {
                            sourceFile = sourceFile.withCharset(pt.getCharset());
                        }
                        return sourceFile
                                .withCharsetBomMarked(pt.isCharsetBomMarked())
                                .withChecksum(pt.getChecksum())
                                .withFileAttributes(pt.getFileAttributes())
                                .withSourcePath(pt.getSourcePath())
                                .withMarkers(pt.getMarkers())
                                .withId(pt.getId());
                    }
                }
                return tree;
            }
        };
        return Preconditions.check(new FindSourceFiles(filePattern), plainTextVisitor);
    }
}

If the above recipe approach is workable then that might then remove the need for an extension mechanism, and the necessary configuration handling downstream to do that well.

[jimschubert]

Thanks @timtebeek for the clarification on the depreciation and the example code! The depreciation message didn't make the clarification you made above, and although I checked the code, I must have missed that a prefixed option was still available.

I'll give your example recipe a try. This is effectively what I'm doing already, as I found that I had to first do a search or other recipe on the file to convert. However I had to put a hack around the parser due to hard-coded extensions: https://github.com/openrewrite/rewrite/blob/98b720b802ea9d4f3d5b6c3e0ce532520c3e311f/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java#L436. I can't recall what was calling the accept method.

I'll return to work around the beginning of August and respond to the issue if my experience with the YAML files has changed after incorporating the above.