Enables plugin to analyze kotlin projects #378

rakshitkr · 2020-02-24T19:57:46Z

Signed-off-by: Rakshit Krishnappa Ravi [email protected]

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

…iggering Kotlin Compiler Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

…is uninstalled Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

kruegers · 2020-02-26T03:21:39Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+ 					if(ip.hasNature("org.eclipse.m2e.core.maven2Nature")) {
+ 						if(KotlinUtils.verifyKotlinDependency(javaProject)) {
+
+ 							System.out.println("Custom Builder started.");


Use Activator.getDefault().logInfo() if you really want to log something.
In this case, be more specific than "custom builder". A user would have no idea, what builder this message is talking about.

kruegers · 2020-02-26T03:21:46Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+ 						if(KotlinUtils.verifyKotlinDependency(javaProject)) {
+
+ 							System.out.println("Custom Builder started.");
+ 							final Job builder = new Job("Custom Builder") {


kruegers · 2020-02-26T03:22:18Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+									try {
+										ip.build(IncrementalProjectBuilder.CLEAN_BUILD, null);
+									} catch (CoreException e) {
+										e.printStackTrace();


All exception logging should go through Activator.getDefault.logError().

kruegers · 2020-02-26T03:22:30Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+ 							while(builder.getState() != Job.NONE) {
+								Thread.sleep(3);
+							}
+ 							System.out.println("Custom Builder finished.");


kruegers · 2020-02-26T03:23:37Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+ 							KotlinUtils.waitForBuildAndRefreshJobs();
+
+							KotlinCompilerResult result = KotlinCompiler.compileKotlinFiles(javaProject);
+							if(result.compiledCorrectly())


Add curly braces, even if there is just one line in the branch.

Also, inline "result". The variable is only used once.

kruegers · 2020-02-26T03:35:51Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+
+ 		if (pomFile != null && pomFile.exists())
+ 		{
+ 			FileReader reader = null;


Rather use try with resource: https://docs.oracle.com/en/java/javase/13/language/try-resources.html

kruegers · 2020-02-26T03:36:39Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+		dialog.setMessage("Analysis requires maven kotlin-stdlib dependency. Would you like to add it?");
+ 		dialog.setText("CAUTION");
+ 		int opt = dialog.open();
+ 		if (opt == SWT.YES)


Change to "return dialog.open();"

kruegers · 2020-02-26T03:37:11Z

plugins/de.cognicrypt.staticanalyzer/.classpath

-	<classpathentry kind="src" path="src"/>
-	<classpathentry kind="output" path="target/classes"/>
-</classpath>
+<?xml version="1.0" encoding="UTF-8"?>


This file should stay as it is.

kruegers · 2020-02-26T03:38:08Z

plugins/de.cognicrypt.staticanalyzer/META-INF/MANIFEST.MF

@@ -23,4 +23,5 @@ Bundle-ActivationPolicy: lazy
 Bundle-ClassPath: .,
 resources/
 Automatic-Module-Name: de.cognicrypt.staticanalyzer
-Import-Package: org.eclipse.ui.texteditor
+Import-Package: de.cognicrypt.staticanalyzer.kotlin.utilities,


If there is a dependency between these two plugins, it should be the other way around. Please check why this dependency exists and resolve it differently.

kruegers · 2020-02-26T03:38:42Z

.../de.cognicrypt.staticanalyzer/src/de/cognicrypt/staticanalyzer/handlers/AnalysisKickOff.java

-		return true;
-	}
+
+		KotlinUtils.compileKotlinFiles(ip);


Do not call the kotlin plugin from inside this one like this. This call causes the kotlin plugin to be required under all circumstances.

Could you please suggest me some way to achieve this? I've implemented rest of your review changes except this one.

I think reflection or some kind of dependency injection would be the way to go. Reflection is not that clean. We should look up some dependency injection frameworks that are clean. This sounds tricky to accomplish but a needed requirement as we do not want to have dependency from kotlin to base.

One option is to use the factory pattern. Have a base factory runner that calls the kotlin runner if configuration is kotlin. https://www.tutorialspoint.com/design_pattern/factory_pattern.htm

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]> Below changes have been made: - replaced sysouts with logInfos - changed exception logging to go through Activator.getDefault.logError() - added curly braces to single line branch code - removed single use variables - used try-with-resource syntax instead of try-finally - changed return type of selectedDialogOption method

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]> Below changes have been made: - replaced sysouts with logInfos - changed exception logging to go through Activator.getDefault.logError() - added curly braces to single line branch code - removed single use variables - used try-with-resource syntax instead of try-finally - changed return type of selectedDialogOption method

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]> - implemented extension - staticanalyzer plugin is the host plugin - kotlin plugin is the extender plugin that gets triggered when the project is of kotlin nature - this would would make staticanalyzer plugin independent of kotlin plugin

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]> - this is done because there is no Activator instance for kotlin plugin since its invoked as a listener by staticanalyzer

rakshitkr · 2020-05-07T11:47:25Z

The sample projects for testing kotlin plugin can be found here

AnakinRaW · 2020-06-16T12:14:51Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+
+ 							while(builder.getState() != Job.NONE) {
+								Thread.sleep(3);
+							}


Do we need to sleep or could we just Thread.yield()?

AnakinRaW · 2020-06-16T12:16:18Z

...ypt.staticanalyzer.kotlin/src/de/cognicrypt/staticanalyzer/kotlin/utilities/KotlinUtils.java

+			}
+		}
+
+		throw new ClassNotFoundException("Class " + className + " not found.");


Either use a CogniCryptException or IOException

AnakinRaW · 2020-06-16T12:18:01Z

plugins/de.cognicrypt.staticanalyzer/src/de/cognicrypt/staticanalyzer/IListener.java

+import org.eclipse.jdt.core.JavaModelException;
+
+public interface IListener {
+	void listen1(IProject ip);


Do we have better names?

AnakinRaW · 2020-06-16T12:19:16Z

plugins/de.cognicrypt.staticanalyzer/src/de/cognicrypt/staticanalyzer/utilities/Utils.java

+ 	        }
+ 		}
+ 		catch (final JavaModelException e) {
+ 			throw new ClassNotFoundException("Class " + className + " not found.", e);


same as above

rakshitkr added 8 commits February 4, 2020 14:33

Enables plugin to analyze kotlin projects

c19a638

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Fix for analyzing eclipse created kotlin-maven-project

3dcbd44

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Implemented Eclipse Job for builder invocation

def16d8

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Added missing KotlinCompiler call for non-maven projects

9db3360

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Implemented wait functionality for all build & refresh jobs before tr…

1e77ecf

…iggering Kotlin Compiler Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Added kotlin jar to get rid of unresolved imports when kotlin plugin …

d1e7bf4

…is uninstalled Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Moved kotlin functionality to separate plugin

f2e87d9

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

Incorporating review comments

496968f

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

rakshitkr requested a review from kruegers February 24, 2020 19:58

kruegers requested changes Feb 26, 2020

View reviewed changes

rakshitkr added 2 commits April 14, 2020 14:11

Restoring changes before commit f2e87d9

e0749a8

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]>

rakshitkr force-pushed the kotlin-fix-v3 branch from 6c94a02 to f0ac0c0 Compare April 14, 2020 12:11

rakshitkr added 4 commits April 14, 2020 14:48

Merge branch 'develop' into kotlin-fix-v3

03a3776

Replaced kotlin.Activator with staticanalyzer.Activator

1676529

Signed-off-by: Rakshit Krishnappa Ravi <[email protected]> - this is done because there is no Activator instance for kotlin plugin since its invoked as a listener by staticanalyzer

Merge branch 'develop' into kotlin-fix-v3

c6b5f2e

kruegers self-requested a review May 6, 2020 16:38

Merge remote-tracking branch 'upstream/develop' into kotlin-fix-v3

d71ba1b

AnakinRaW suggested changes Jun 16, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enables plugin to analyze kotlin projects #378

Enables plugin to analyze kotlin projects #378

rakshitkr commented Feb 24, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

kruegers Feb 26, 2020

rakshitkr Mar 14, 2020

krinara86 Mar 25, 2020

krinara86 Mar 25, 2020

rakshitkr commented May 7, 2020 •

edited

Loading

AnakinRaW Jun 16, 2020

AnakinRaW Jun 16, 2020

AnakinRaW Jun 16, 2020

AnakinRaW Jun 16, 2020

Enables plugin to analyze kotlin projects #378

Are you sure you want to change the base?

Enables plugin to analyze kotlin projects #378

Conversation

rakshitkr commented Feb 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakshitkr commented May 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakshitkr commented May 7, 2020 •

edited

Loading