Skip to content

Conversation

@JakeWharton
Copy link
Collaborator

@JakeWharton JakeWharton commented Apr 27, 2023

Trying to pull out self-contained bits of this to start landing. Otherwise the final PR would be like 10,000 changed lines.

Refs #19

Comment on lines 50 to 57
interface NonAnnotationSchema

@Test fun nonAnnotatedSchemaThrows() {
assertFailsWith<IllegalArgumentException> {
parseProtocolSchemaSet(NonAnnotationSchema::class)
parser.parse(NonAnnotationSchema::class)
}.hasMessage(
"Schema app.cash.redwood.tooling.schema.SchemaParserTest.NonAnnotationSchema missing @Schema annotation",
)
Copy link
Collaborator Author

@JakeWharton JakeWharton Apr 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only test that's working in FIR (in this PR). It means we can successfully look up the schema type and can notice that it's missing annotations.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@JakeWharton JakeWharton force-pushed the jw.schema-parser-variation.2023-04-26 branch 2 times, most recently from d4f3eb9 to c9f1935 Compare April 27, 2023 19:46
@JakeWharton
Copy link
Collaborator Author

Well well well. This is interesting. So our CLI depends on the schema parser which gained an embeddable compiler dependency but also depends on AGP lint which has its own embeddable compiler dependency (but an older one). And due to the general instability of the compiler's API, any version deviation breaks the other.

I think for now I'll just match lint's embeddable compiler version. Long term we probably want to eliminate the centralized CLI and instead have each tooling module provide its own CLI. This would have the parser exposing a JSON-producing command. The lint tooling would expose the lint command and the API merge command. And the code generator exposing the generate command.

This is ideal because the Gradle plugin loads these in individual classpaths and forks a JVM to execute each tool. Thus, they would not longer interfere with each other.

I'll do that next since it's pretty easy.

@JakeWharton
Copy link
Collaborator Author

Nope. Won't work. Versions are too far apart. Doing the other thing first.

@JakeWharton JakeWharton force-pushed the jw.schema-parser-variation.2023-04-26 branch from c9f1935 to a0c96a8 Compare April 28, 2023 12:37
@JakeWharton JakeWharton marked this pull request as ready for review April 28, 2023 12:38
@JakeWharton
Copy link
Collaborator Author

Might need to change the Lint worker isolation to prevent the dependency clash. We'll see.


// In order to simplify writing test schemas, inject the test sources and
// test classpath as properties into the test runtime. This allows testing
// the FIR-based parser on sources written inside the test case. Cool!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COOL!

@JakeWharton JakeWharton merged commit 9bb3098 into trunk May 4, 2023
@JakeWharton JakeWharton deleted the jw.schema-parser-variation.2023-04-26 branch May 4, 2023 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants