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

New Test Framework #236

Merged
merged 13 commits into from
Aug 15, 2023
Merged

New Test Framework #236

merged 13 commits into from
Aug 15, 2023

Conversation

Feuermagier
Copy link
Owner

@Feuermagier Feuermagier commented Jul 9, 2023

See Test_Framework.md for a description. All tests except ImportTypes work, which fails because the new framework is always using virtual files and Spoon cannot get the original source code for virtual files. I've fixed some other problems along the way such as the StringSourceInfo not being serializable.

…o the virtual files for which spoon cannot get the original source code
@Feuermagier Feuermagier self-assigned this Jul 9, 2023
@Luro02
Copy link
Collaborator

Luro02 commented Jul 10, 2023

That is going to be a pain to merge with my PR

@Luro02
Copy link
Collaborator

Luro02 commented Jul 10, 2023

The problem with ImportTypes has been fixed through my PR, which enables accessing the SourceInfo directly and by that the source code

Copy link
Collaborator

@Luro02 Luro02 left a comment

Choose a reason for hiding this comment

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

Looks good to me, a bit of documentation is missing for the //?# syntax.
I can fix the import types lint, after you rebase (or merge) #231.

Before my PR it was only possible to access the source code through the Spoon API which is very fragile.

StringSourceInfo merge conflict

I made a few bug fixes in StringSourceInfo so replacing it with your version is not recommended. For fixing the mentioned bugs in your PR, I merged mine.

The Serialization issue might be fixed by using the new API that has a

        @Override
        public JavaFileObject toJavaFileObject() {
            return this;
        }

method and return a new object instead of this:

private static final class VirtualJavaFileObject extends SimpleJavaFileObject {
    private final VirtualFileObject virtualFileObject;

    public VirtualJavaFileObject(URI uri, Kind kind, VirtualFileObject virtualFileObject) {
        super(uri, kind);
        this.virtualFileObject = virtualFileObject;
    }

    @Override
    public String getName() {
        return this.virtualFileObject.path().toString();
    }

    @Override
    public CharSequence getCharContent(boolean ignoreEncodingErrors) {
        return this.virtualFileObject.getCode();
    }
}

private static final class VirtualFileObject implements Serializable, CompilationUnit {
        private final ClassPath classPath;
        private final String code;
...

        @Override
        public JavaFileObject toJavaFileObject() {
            return new VirtualJavaFileObject(virtualUri(this.classPath), Kind.SOURCE, this);
        }
...
}

For the long term it will be better to not use inheritance and instead implement the JavaFileObject interface.

Most methods are a no-op implementation in SimpleJavaFileObject. This is what I did for the PhysicalFileObject class:

public class PhysicalFileObject implements CompilationUnit, JavaFileObject {
private final File file;
private final SourcePath path;
private final SerializableCharset charset;
public PhysicalFileObject(File file, Charset charset, SourcePath path) {
this.file = file;
this.path = path;
this.charset = new SerializableCharset(charset);
}
@Override
public String getName() {
// NOTE: PMD relies on this method to return something that looks like a path
return this.path().toString();
}
@Override
public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOException {
return Files.readString(this.file.toPath(), this.charset);
}
@Override
public OutputStream openOutputStream() throws IOException {
Files.createDirectories(this.file.getParentFile().toPath());
try {
Files.createFile(this.file.toPath());
} catch (FileAlreadyExistsException ignored) {
// ignored
}
return new FileOutputStream(this.file);
}
@Override
public JavaFileObject toJavaFileObject() {
return this;
}
@Override
public Charset charset() {
return this.charset;
}
@Override
public SourcePath path() {
return this.path;
}
// This is mostly copy-pasted from SimpleJavaFileObject implementation
@Override
public URI toUri() {
return this.file.toURI();
}
@Override
public InputStream openInputStream() throws IOException {
throw new UnsupportedOperationException();
}
@Override
public Reader openReader(boolean ignoreEncodingErrors) throws IOException {
return new StringReader(this.getCharContent(ignoreEncodingErrors).toString());
}
@Override
public Writer openWriter() throws IOException {
// ensure that the correct charset is used
return new OutputStreamWriter(this.openOutputStream(), this.charset());
}
@Override
public long getLastModified() {
return 0L;
}
@Override
public boolean delete() {
return false;
}
@Override
public Kind getKind() {
return Kind.SOURCE;
}
@Override
public boolean isNameCompatible(String simpleName, Kind kind) {
String baseName = simpleName + kind.extension;
return kind == this.getKind()
&& (baseName.equals(this.toUri().getPath())
|| this.toUri().getPath().endsWith("/" + baseName));
}
@Override
public NestingKind getNestingKind() {
return null;
}
@Override
public Modifier getAccessLevel() {
return null;
}
}

and this is the only copied method from SimpleJavaFileObject that is non-trivial:
public boolean isNameCompatible(String simpleName, Kind kind) {
String baseName = simpleName + kind.extension;
return kind == this.getKind()
&& (baseName.equals(this.toUri().getPath())
|| this.toUri().getPath().endsWith("/" + baseName));
}

I am not sure what this method does, might be possible to simplify it by directly accessing the path instead of the URI

@Feuermagier
Copy link
Owner Author

VirtualFileObject now directly implements JavaFileObject; this fixed the serialization issue

@Luro02 Luro02 merged commit 7d70ed5 into main Aug 15, 2023
4 checks passed
@Luro02 Luro02 deleted the test-framework branch August 15, 2023 07:04
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.

2 participants