kontext-e / jqassistant-plugins

This repository contains plugins for buschmais' jQAssistant.
GNU General Public License v3.0
18 stars 10 forks source link

Plugins shall migrate existing file nodes instead of creating new ones #59

Closed DirkMahler closed 6 years ago

DirkMahler commented 6 years ago

Currently the plugin implementations create new File labeled nodes instead of using the ones already provided by the file scanner:

    public JavaSourceDescriptor scan(final FileResource item, final String path, final Scope scope, final Scanner scanner) throws IOException {
        final Store store = scanner.getContext().getStore();
        final JavaSourceFileDescriptor javaSourceFileDescriptor = store.create(JavaSourceFileDescriptor.class);
        javaSourceFileDescriptor.setFileName(path);

Instead the following pattern should be used:


@ScannerPlugin.Requires(FileDescriptor.class)
public class JavaparserScannerPlugin extends AbstractScannerPlugin<FileResource, JavaSourceDescriptor> {

....
    public JavaSourceDescriptor scan(final FileResource item, final String path, final Scope scope, final Scanner scanner) throws IOException {
        final Store store = scanner.getContext().getStore();
        FileDescriptor fileDescriptor = scanner.getContext().getCurrentDescriptor();
        final JavaSourceFileDescriptor javaSourceFileDescriptor = store.migrate(fileDescriptor, JavaSourceFileDescriptor.class);
...
kontext-e commented 6 years ago

Will be in 1.4.1

kontext-e commented 6 years ago

Did 'migrate' instead of 'create', but it does not work because Labels vanish. E.g. when the linecount plugin runs after this plugin (which accepts all files), there is only the Linecount label left. Is this the expected behavior? I expected that all label accumulate instead replace. With current behavior 'migrate' is useless when several plugins run after each other.

DirkMahler commented 6 years ago

Migrate is already heavily used by other plugins to add labels, see https://github.com/buschmais/jqa-java-plugin/blob/master/src/main/java/com/buschmais/jqassistant/plugin/java/impl/scanner/PropertyFileScannerPlugin.java#L41

kontext-e commented 6 years ago

Ah, but this is 'addDescriptorType' which I expect to work like desired. Will give it a try.

kontext-e commented 6 years ago

Seems to work as expected this time.