4
\$\begingroup\$

I have written Pycharm plugin for integrating pycharm with tidyimports. I have followed steps mentioned in Simple Pycharm Plugin Example.

Below is the code that I have written, it works but I need this to be reviewed.

1.Tidy Import

import com.intellij.openapi.actionSystem.AnAction;
import com.intellij.openapi.actionSystem.AnActionEvent;
import com.intellij.openapi.actionSystem.LangDataKeys;
import com.intellij.openapi.application.Application;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.editor.Document;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;


public class TidyImportAction extends AnAction
{
    @Override
    public void actionPerformed(AnActionEvent e)
    {
        Document document = e.getData(LangDataKeys.EDITOR).getDocument();
        String source_code = document.getText();
        source_code = source_code.replace("\"","\\\"");
        String[] cmd = {
                "/bin/sh",
                "-c",
                String.format("echo \"%s\" | tidy-imports", source_code)
        };

        Process p = null;
        try {
            p = Runtime.getRuntime().exec(cmd);

            BufferedReader stdInput = new BufferedReader(new
                    InputStreamReader(p.getInputStream()));

            BufferedReader stdError = new BufferedReader(new
                    InputStreamReader(p.getErrorStream()));

            String modified_source_code = "";
            String pyflyby_output = "";
            String s = "";

            while ((s = stdInput.readLine()) != null) {

                if (s.startsWith("[PYFLYBY]")){
                    if (s != null)
                        pyflyby_output += s + "\n";

                } else {
                    if (s != null)
                        modified_source_code += s + "\n";
                }

            }

            while ((s = stdError.readLine()) != null) {
                if (s.startsWith("[PYFLYBY]")){
                    if (s != null)
                        pyflyby_output += s;

                } else {
                    if (s != null)
                        modified_source_code += s;
                }
            }

            Application application = ApplicationManager.getApplication();
            String finalModified_source_code = modified_source_code;
            application.runWriteAction(() -> {
                document.setText(finalModified_source_code);
            });


        } catch (IOException e1) {
            e1.printStackTrace();
        }
    }
}

2.Hot Import

import com.intellij.openapi.actionSystem.*;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.ui.Messages;
import com.intellij.openapi.vfs.*;
import com.intellij.psi.*;
import com.jetbrains.python.psi.PyExpression;
import com.jetbrains.python.psi.PyExpressionStatement;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.uast.values.UBooleanConstant;

import java.io.*;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;

import com.jetbrains.python.*;


public class HotImportsAction extends AnAction {

    Boolean hotimports_enabled = false;

    public HotImportsAction() {
        super("Enable HotImports");
        System.out.println("Registering file watch");
//        VirtualFileManager

        VirtualFileManager.getInstance().addVirtualFileListener(new VirtualFileListener() {
            @Override
            public void contentsChanged(VirtualFileEvent event) {
                VirtualFile virtual_file = event.getFile();
                System.out.println("contentsChanged: " + virtual_file.toString());

                if (virtual_file.toString().endsWith(".py") != true) {
                    return;
                }

                System.out.println("hotimports_enabled = " + hotimports_enabled.toString());
                // do something
                if (hotimports_enabled == true) {
                    System.out.println("File Changed: " + virtual_file.toString());

                    DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");
                    Date date = new Date();
                    System.out.println(dateFormat.format(date)); //2016/11/16 12:08:43

                    String file_path = event.getFile().getPath();

                    run_tidyimports(file_path);
                    virtual_file.refresh(false, false);
                }
            }
        });
    }

    public void actionPerformed(AnActionEvent event) {
        Presentation presentation = event.getPresentation();
        if (hotimports_enabled == true) {
            presentation.setText("Enable HotImports");
            hotimports_enabled = false;
        } else {
            run_tidyimports(event.getData(PlatformDataKeys.VIRTUAL_FILE).getPath());
            presentation.setText("Disable HotImports");
            hotimports_enabled = true;
        }

        PsiFile psi_file = event.getData(LangDataKeys.PSI_FILE);
//        print_expressions(psi_file);

        VirtualFile vFile = event.getData(PlatformDataKeys.VIRTUAL_FILE);
        vFile.refresh(false, false);
    }

    public void print_expressions(PsiFile psi_file) {
        ArrayList<String> knownExprs = new ArrayList<String>();

        if (psi_file != null) {
            psi_file.accept(new PsiRecursiveElementVisitor() {
                @Override
                public void visitElement(PsiElement element) {
//                System.out.println("PsiElement: " + element.toString());
                    if (element instanceof PyExpressionStatement) {
                        System.out.println("PyExpressionStatemt: " + element.toString() + " " + element.getText());
                        knownExprs.add(element.getText());
                    }
                    super.visitElement(element);
                }
            });
        }
        System.out.println("knownExpr: " + knownExprs.toString());
    }

    public static void run_tidyimports(String filePath) {
        System.out.println("Running tidy-imports on " + filePath);
        if (filePath == null || !filePath.endsWith(".py")) {
            return;
        }
        String s = null;
        try {
            Process p = Runtime.getRuntime().exec("tidy-imports -r " + filePath);
            BufferedReader stdInput = new BufferedReader(new InputStreamReader(p.getInputStream()));
            BufferedReader stdError = new BufferedReader(new InputStreamReader(p.getErrorStream()));
            System.out.println("Here is the standard output of the command:\n");
            while ((s = stdInput.readLine()) != null) {
                System.out.println(s);
            }
            System.out.println("Here is the standard error of the command (if any):\n");
            while ((s = stdError.readLine()) != null) {
                System.out.println(s);
            }
        }
        catch (Exception e) {
            System.out.println("exception happened - here's what I know: ");
            e.printStackTrace();
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Vogel612 2 days ago
2
\$\begingroup\$

General

In idiomatic Java, open curly braces belong on the same line, not a newline.

In idiomatic Java, variable names use camelCase. Underscores are only used when naming constants.

Classes not designed for extension should be marked as final. Variables that will not be reassigned should be marked as final. This clarifies the design intent of the author and makes it easier to read the code, because you have a compiler-enforced guarantee against change/extension.

Curly braces should always be used, even when they're optional. They make it easier to read the code and harder to introduce bugs when modifying the code later.

If you haven't read the excellent JavaWorld article on Runtime.exec(), you should. https://www.javaworld.com/article/2071275/when-runtime-exec---won-t.html

You may also consider using ProcessBuilder. It's a little easier to read and is more flexible. https://docs.oracle.com/javase/7/docs/api/java/lang/ProcessBuilder.html

If IntelliJ has some framework for logging, that may be preferable to writing directly to System.out and System.err (with e.printStackTrace()). This is doubly true if, as it appears, your System.out calls are intended for debugging purposes.

TidyImportAction

You should probably add a no-arg constructor so you can specify the action name.

pyflyby_output is built, but never used. As is, it should be removed.

p is a poor name for a variable. process would be better.

p can be defined inside the try block.

Your null checks do nothing. If s was null, you'd break out of the while loop. If you somehow got past that, you'd get a NullPointerException when checking if it started with [PYFLYBY].

Modifying a String is expensive. Use the mutable StringBuilder instead. The compiler is probably doing this for you behind the scenes.

There's no value to finalModified_source_code. Just use the String value.

HotImportsAction

hotimports_enabled is a Boolean (object), but can never be null. You should prefer boolean (primitive). It's slightly more efficient, and also makes it clear that it cannot ever be null.

hotimports_enabled should be private, as it's not intended to be available outside HotImportsAction.

Don't explicitly compare boolean values to true and false. Just use if (whatever), not if (whatever == true).

Delete commented-out code. All of your non-code comments are really just visual noise and can also be removed.

Guard clauses make your code less indented in general, and thus easier to read.

actionPerformed should be annotated with @Override.

psi_file is unused and should be removed. Likewise print_expressions.

Catch the most specific exception you can. In particular, catch IOException instead of Exception.


If you were to make all of these changes, your code might look more like:

public final class TidyImportAction extends AnAction {

    public TidyImportAction() {
        super("Tidy Import");
    }

    @Override
    public void actionPerformed(final AnActionEvent event) {
        final Document document = event.getData(LangDataKeys.EDITOR).getDocument();
        final String sourceCode = document.getText().replace("\"","\\\"");
        final String[] cmd = {
                "/bin/sh",
                "-c",
                String.format("echo \"%s\" | tidy-imports", sourceCode)
        };

        try {
            final Process process = Runtime.getRuntime().exec(cmd);
            final BufferedReader standardInput = new BufferedReader(new InputStreamReader(process.getInputStream()));
            final BufferedReader standardError = new BufferedReader(new InputStreamReader(process.getErrorStream()));
            final StringBuilder modifiedSourceCode = new StringBuilder();

            String s = "";
            while ((s = standardInput.readLine()) != null) {
                if (!s.startsWith("[PYFLYBY]")) {
                    modifiedSourceCode.append(s).append("\n");
                }
            }

            while ((s = standardError.readLine()) != null) {
                if (!s.startsWith("[PYFLYBY]")) {
                    modifiedSourceCode.append(s);
                }
            }

            ApplicationManager.getApplication().runWriteAction(() -> {
                document.setText(modifiedSourceCode.toString());
            });


        } catch (final IOException e) {
            e.printStackTrace();
        }
    }

}

public final class HotImportsAction extends AnAction {

    private boolean hotImportsEnabled = false;

    public HotImportsAction() {
        super("Enable HotImports");
        System.out.println("Registering file watch");

        VirtualFileManager.getInstance().addVirtualFileListener(new VirtualFileListener() {
            @Override
            public void contentsChanged(final VirtualFileEvent event) {
                final VirtualFile virtualFile = event.getFile();
                System.out.println("contentsChanged: " + virtualFile.toString());

                if (!virtualFile.toString().endsWith(".py")) {
                    return;
                }

                System.out.println("hotImportsEnabled = " + hotImportsEnabled);
                if (!hotImportsEnabled) {
                    return;
                }

                System.out.println("File Changed: " + virtualFile.toString());

                final DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");
                System.out.println(dateFormat.format(new Date()));

                final String filePath = event.getFile().getPath();

                runTidyImports(filePath);
                virtualFile.refresh(false, false);
            }
        });
    }

    @Override
    public void actionPerformed(final AnActionEvent event) {
        final Presentation presentation = event.getPresentation();
        if (hotImportsEnabled) {
            presentation.setText("Enable HotImports");
        } else {
            runTidyImports(event.getData(PlatformDataKeys.VIRTUAL_FILE).getPath());
            presentation.setText("Disable HotImports");
        }
        hotImportsEnabled = !hotImportsEnabled;

        event.getData(PlatformDataKeys.VIRTUAL_FILE).refresh(false, false);
    }

    public static void runTidyImports(final String filePath) {
        System.out.println("Running tidy-imports on " + filePath);
        if (filePath == null || !filePath.endsWith(".py")) {
            return;
        }
        try {
            final Process p = Runtime.getRuntime().exec("tidy-imports -r " + filePath);
            final BufferedReader standardInput = new BufferedReader(new InputStreamReader(p.getInputStream()));
            final BufferedReader standardError = new BufferedReader(new InputStreamReader(p.getErrorStream()));

            System.out.println("Here is the standard output of the command:\n");
            String s = null;
            while ((s = standardInput.readLine()) != null) {
                System.out.println(s);
            }
            System.out.println("Here is the standard error of the command (if any):\n");
            while ((s = standardError.readLine()) != null) {
                System.out.println(s);
            }
        } catch (final IOException e) {
            System.out.println("exception happened - here's what I know: ");
            e.printStackTrace();
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.