Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse SPI configuration files as PlainText #344

Merged
merged 5 commits into from
May 18, 2022

Conversation

m-brophy
Copy link
Contributor

@m-brophy m-brophy commented Apr 29, 2022

…ecipes to run against them.

The purpose of this is to enable the running of recipe org.openrewrite.RenameFile against bootstrapping files in a javax-jakarta migration like so:

---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.jakarta.javax.BootstrappingFiles
displayName: migrate name of bootstrapping files
recipeList:
    - org.openrewrite.RenameFile:
          fileName: jakarta.enterprise.inject.spi.Extension
          fileMatcher: "**/javax.enterprise.inject.spi.Extension"

I don't know if you think it necessary to restrict the files found by the accept method of PlainTextParser but that could be achieved for my purposes by adding any files in path resources/META-INF/services as well as files with appropriate extensions.

@tkvangorder tkvangorder self-assigned this Apr 29, 2022
@tkvangorder
Copy link
Contributor

@m-brophy

After talking it over with the team, we felt we needed a better, generalized solution to handling "files that are known to exist but are not formally parsed by one of rewrite's parser".

The problem with using the PlainText parser to scoop up everything that is left over is that it will likely encounter binary files that would result in exceptions when attempting to use a character encoding.

I am going to close this PR but know that your input resulted in a "new type of file that exists but cannot be seen". 8)

openrewrite/rewrite#1734

We landed on "quark" because we are all nerds.

We will prioritize this into our backlog and try to get this work done soon.

Thanks again!

@tkvangorder
Copy link
Contributor

@m-brophy I have created this issue to add the support that you are looking for!

#346

Once this is in place, you can use the new visitSourceFile() method to alter/rename the path.

Again, thanks for you input!

@jkschneider
Copy link
Member

@m-brophy Just want to provide a follow-up on this. In rewrite-core, I introduced an odd duck: a Quark. It's a SourceFile implementation that has no content (only a source path and markers). The build tools in this next release will be configured to add a Quark instance for every file in the repository that (1) isn't gitignored and (2) isn't parsed into a contentful SourceFile implementation like a J or Xml.

We'll update RenameFile to be able to operate on every source file, including a Quark. In essence, it is possible for us to rename a file without ever seeing its contents (so similar capability without the potential downside of memory consumption for things we don't otherwise need to parse).

I would appreciate this PR still, but slightly modified -- I'd still like to parse these particular files as PlainText (anything that can be loaded by a ServiceLoader.

@jkschneider jkschneider reopened this May 1, 2022
@jkschneider
Copy link
Member

jkschneider commented May 1, 2022

Quarks give us the ability to see everything that isn't parsed, but let's still parse META-INF/services/** as PlainText. This is especially important for frameworks that more heavily use SPI like Quarkus.

@jkschneider jkschneider changed the title enableTextFileRename: load files found by PlainTextParser to enable r… Parse SPI configuration files as PlainText May 1, 2022
@tkvangorder tkvangorder removed their assignment May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants