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

Java: Add test for a JavacTool-based compiler that doesn't use standard JavaFileObjects #18305

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

smowton
Copy link
Contributor

@smowton smowton commented Dec 17, 2024

This tests the situation where a JavacTool-based compiler doesn't just refer to standard files on disk, but rather directly refers to source files in a JAR or other archive, or as in the case tested here, uses an in-memory file object backed by a buffer, not a file.

The Java agent previously couldn't deal with this, but should now read such files out as text and write them to a temporary directory before extracting them.

@Copilot Copilot bot review requested due to automatic review settings December 17, 2024 14:43
@smowton smowton requested a review from a team as a code owner December 17, 2024 14:43
@github-actions github-actions bot added the Java label Dec 17, 2024

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • java/ql/integration-tests/java/javac-tool-custom-file/test.expected: Language not supported
  • java/ql/integration-tests/java/javac-tool-custom-file/test.ql: Language not supported

Tip: Turn on automatic Copilot reviews for this repository to get quick feedback on every pull request. Learn more

@smowton smowton force-pushed the smowton/admin/agent-extracted-file-test branch from 4ec706a to 898995f Compare December 18, 2024 15:29
@smowton smowton force-pushed the smowton/admin/agent-extracted-file-test branch from 898995f to c030d88 Compare January 8, 2025 14:52
@@ -0,0 +1 @@
| Main |
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this get some kind of location based on https://nonesuch.imaginary/somedir/Main.java?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently the loc seen will be inside the db's scratch dir where the temporary is created. Almost necessarily the loc given by toURI or similar is going to be something vscode etc won't understand, like a jar-relative URI or the like; at least this location will be clickable, though considered outside the source-root. I haven't exposed it in this test because it isn't stable from test run to run (contains a temporary dirname).

I suggest we don't tackle this one at this juncture, since I'm not clear how we can give it a more useful location that makes sense to VSCode / another client, or the user.

@smowton smowton force-pushed the smowton/admin/agent-extracted-file-test branch from c030d88 to 060161c Compare January 14, 2025 14:22
@smowton smowton merged commit b2bb143 into github:main Jan 14, 2025
10 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants