Skip to content

fix compatibility with IDEA 2016.1 and above #186

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.

Sign up for GitHub

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

Open
TomDevs wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix compatibility with IDEA 2016.1 and above #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f279285
Add import block name config
TomDevs Nov 28, 2017
3a12399
Compatibility with IntelliJ IDEA 2017.3
TomDevs Nov 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions META-INF/plugin.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<idea-plugin version="2">
<id>com.chrisfolger.needsmoredojo</id>
<name>Needs More Dojo</name>
<version>0.7</version>
<version>0.8.1</version>
Copy link

@sindilevich sindilevich Dec 19, 2017

Choose a reason for hiding this comment

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

#181 suggests moving the file to resources/META-INF/plugin.xml and updating resources/META-INF/MANIFEST.MF.

Copy link
Author

@TomDevs TomDevs Dec 21, 2017

Choose a reason for hiding this comment

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

I don't think this file location particularly matters, but if this becomes a problem I'll happily move it.

<vendor email="cefolger@gmail.com" url="http://www.chrisfolger.com">Chris Folger</vendor>

<description><![CDATA[
Expand Down Expand Up @@ -40,6 +40,17 @@
]]></description>

<change-notes><![CDATA[
<b>0.8.1</b>
<ul>
<li>Compatibility with IntelliJ IDEA 2017.3</li>
</ul>
<b>0.8</b>
<ul>
<li>Compatibility with IntelliJ IDEA 2016.1 and above</li>
<li>Added import block name configuration option</li>
</ul>
<b>0.7 (major release) </b>
<ul>
<li>Quick fix to add a mismatched import exception </li>
Expand Down Expand Up @@ -89,7 +100,7 @@
]]>
</change-notes>

<idea-version since-build="107.105"/>
<idea-version since-build="145.1"/>

<depends>JavaScript</depends>
<depends>com.intellij.modules.xml</depends>
Expand Down Expand Up @@ -217,7 +228,7 @@

<!-- overrides the default JavaScript unresolved variable inspection so that we are able to resolve attach points. Otherwise there
is no way to provide references to attach point variables -->
<localInspection language="JavaScript" groupPath="JavaScript" shortName="JSUnresolvedVariable"
<localInspection language="JavaScript" groupPath="JavaScript"
key="js.unresolved.variable.inspection.name" groupKey="js.inspection.group.name" enabledByDefault="true"
level="WEAK WARNING" implementationClass="com.chrisfolger.needsmoredojo.intellij.inspections.DojoUnresolvedVariableInspection" unfair="true"/>
</extensions>
Expand Down
11 changes: 11 additions & 0 deletions changelog.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
<html>
<body>
<b>0.8.1</b>
<ul>
<li>Compatibility with IntelliJ IDEA 2017.3</li>
</ul>

<b>0.8</b>
<ul>
<li>Compatibility with IntelliJ IDEA 2016.1 and above</li>
<li>Added import block name configuration option</li>
</ul>

<b>0.7 (major release) </b>
<ul>
<li>Quick fix to add a mismatched import exception </li>
Expand Down
Binary file added dist/needsmoredojo-0.8.1.jar
View file
Open in desktop
Binary file not shown.
Binary file added dist/needsmoredojo-0.8.jar
View file
Open in desktop
Binary file not shown.
20 changes: 17 additions & 3 deletions src/com/chrisfolger/needsmoredojo/core/amd/define/DefineResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

import com.chrisfolger.needsmoredojo.core.amd.CompletionCallback;
import com.chrisfolger.needsmoredojo.core.amd.importing.InvalidDefineException;
import com.chrisfolger.needsmoredojo.core.settings.DojoSettings;
import com.intellij.lang.javascript.psi.*;
import com.intellij.openapi.components.ServiceManager;
import com.intellij.openapi.project.Project;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiElementVisitor;
import com.intellij.psi.PsiFile;
Expand Down Expand Up @@ -183,8 +186,8 @@ public void run(Object[] result) {
if(parent instanceof JSCallExpression)
{
JSCallExpression statement = (JSCallExpression) parent;
if(statement.getMethodExpression() != null && (statement.getMethodExpression().getText().equals("define")
|| statement.getMethodExpression().getText().equals("require")))
if(statement.getMethodExpression() != null &&
isValidImportBlockName(statement.getMethodExpression().getText(), element.getProject()))
{
return getDefineStatementItemsFromArguments(statement.getArguments(), statement);
}
Expand Down Expand Up @@ -238,7 +241,7 @@ public Set<JSCallExpression> getAllImportBlocks(PsiFile file)
public void visitJSCallExpression(JSCallExpression expression)
{
if(expression != null && expression.getMethodExpression() != null &&
(expression.getMethodExpression().getText().equals("define") || expression.getMethodExpression().getText().equals("require")))
isValidImportBlockName(expression.getMethodExpression().getText(), file.getProject()))
{
listOfDefinesOrRequiresToVisit.add(expression);
}
Expand All @@ -250,4 +253,15 @@ public void visitJSCallExpression(JSCallExpression expression)

return listOfDefinesOrRequiresToVisit;
}

private Boolean isValidImportBlockName(String name, Project project) {
DojoSettings settings = ServiceManager.getService(project, DojoSettings.class);
String[] defineBlockNames = settings.getImportBlockNames().split(",");
for (String defineBlockName:defineBlockNames) {
if (name.equals(defineBlockName)) {
return true;
}
}
return false;
}
Copy link

@sindilevich sindilevich Dec 19, 2017

Choose a reason for hiding this comment

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

Should also line#47 be changed to Collections.addAll(parameters, function.getParameterVariables());?

}
10 changes: 5 additions & 5 deletions src/com/chrisfolger/needsmoredojo/core/amd/importing/ImportCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ private void displayDuplicateMessage(String parameter, Project project)

public void createImport(String module, String quoteCharacter, String parameter, JSArrayLiteralExpression imports, JSParameterList parameters)
{
for(JSParameter element : parameters.getParameters())
for(JSParameter element : parameters.getParameterVariables())
{
if(element.getName().equals(parameter))
{
Expand All @@ -37,7 +37,7 @@ public void createImport(String module, String quoteCharacter, String parameter,
}
}

if(imports.getChildren().length == 0)
if(imports.getExpressions().length == 0)
Copy link

@sindilevich sindilevich Dec 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

What is the benefit of using getExpressions() over getChildren() here? #181 does not changed that, so am asking for general knowledge.

Copy link
Author

@TomDevs TomDevs Dec 21, 2017

Choose a reason for hiding this comment

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

There was a problem that when adding an import (Ctrl+Shift+O, 2), the import was added in the wrong location, usually outside of the JavaScript import array. Using getExpressions() fixed this issue.

{
// how to insert
/*
Expand Down Expand Up @@ -80,7 +80,7 @@ else if(defineText.contains("\n"))
else
{
String formatString = quoteCharacter + "%s" + quoteCharacter + ",";
JSUtil.addStatementBeforeElement(imports, imports.getChildren()[0], String.format(formatString, module), "\n");
JSUtil.addStatementBeforeElement(imports, imports.getExpressions()[0], String.format(formatString, module), "\n");
if(parameters.getChildren().length > 0)
{
JSUtil.addStatementBeforeElement(parameters, parameters.getChildren()[0], parameter + ",", " ");
Expand All @@ -99,9 +99,9 @@ public void createImport(String module, String quoteCharacter, JSArrayLiteralExp

// if the parameter would cause a duplicate, then assume it is a module with a different path but the same name
// as an existing imported module.
for (int i = 0; i < parameters.getParameters().length; i++)
for (int i = 0; i < parameters.getParameterVariables().length; i++)
{
JSParameter existingParameter = parameters.getParameters()[i];
JSParameter existingParameter = parameters.getParameterVariables()[i];
if(existingParameter != null && existingParameter.getText().equals(parameter))
{
String existingAbsolutePath = NameResolver.getModuleAndPathWithoutPluginResourceId(imports.getExpressions()[i].getText().replaceAll("'", "").replaceAll("\"", ""));
Expand Down
2 changes: 1 addition & 1 deletion src/com/chrisfolger/needsmoredojo/core/amd/importing/ImportReorderer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void doSwap(PsiElement source, Editor editor, AMDPsiUtil.Direction direct

int sourceIndex = PsiUtil.getIndexInParent(defines[0]);
int destinationIndex = PsiUtil.getIndexInParent(defines[1]);
JSParameter[] parameterList = items.getFunction().getParameters();
JSParameter[] parameterList = items.getFunction().getParameterVariables();
Copy link

@sindilevich sindilevich Dec 19, 2017

Choose a reason for hiding this comment

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

Also src/com/chrisfolger/needsmoredojo/core/amd/define/NearestAMDImportLocator.java still use the old getFunction().getParameters() in line 60 and line 123.

And src/com/chrisfolger/needsmoredojo/core/amd/importing/ImportUpdater.java still use the old getFunction().getParameters() in line 65 and line 74.

Copy link
Author

@TomDevs TomDevs Dec 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

In a lot of cases getParameters() and getParameterVariables() can be used to do the same things, so it shouldn't really matter which method is used; I changed the ones necessary to fix the test cases and get the plugin to a working state.

Copy link

@sindilevich sindilevich Dec 21, 2017

Choose a reason for hiding this comment

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

@TomDevs, thank you fro your reply. You may be right, replacing the minimum possible, but there was a problem reported here: #181 (comment) that was handled by replacing getFunction().getParameters() with getFunction().getParameterVariables(), as per 427bc53. That's why I wanted to point out these places to you to look closer at.


if(sourceIndex >= parameterList.length || destinationIndex >= parameterList.length)
{
Expand Down
2 changes: 1 addition & 1 deletion ...om/chrisfolger/needsmoredojo/core/amd/importing/visitors/UnusedImportsRemovalVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private boolean isOwnedByCurrentImportBlock(PsiElement node, String nodeText, St
{
// if the nearest block contains a reference AND it is not equal to the owning block, then the owning
// block cannot claim this reference
for(JSParameter parameter : block.getFunction().getParameters())
for(JSParameter parameter : block.getFunction().getParameterVariables())
{
String matchText = prefix + parameter.getText();

Expand Down
4 changes: 2 additions & 2 deletions src/com/chrisfolger/needsmoredojo/core/amd/psi/AMDPsiUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,9 @@ public static PsiElement resolveReferencedDefine(PsiElement psiElement)

try
{
for (int x = 0; x < defineStatement.getFunction().getParameters().length; x++)
for (int x = 0; x < defineStatement.getFunction().getParameterVariables().length; x++)
{
JSParameter parameter = defineStatement.getFunction().getParameters()[x];
JSParameter parameter = defineStatement.getFunction().getParameterVariables()[x];
JSExpression define = defineStatement.getArguments().getExpressions()[x];

if(parameter.getText().equals(psiElement.getText()))
Expand Down
5 changes: 3 additions & 2 deletions src/com/chrisfolger/needsmoredojo/core/refactoring/UtilFinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ else if (statement instanceof JSVarStatement && statement.getText().contains("de

for(JSVariable variable : ((JSVarStatement)statement).getVariables())
{
if(variable.getInitializerText().contains("declare"))
final JSExpression initializer = variable.getInitializer();
if(initializer instanceof JSCallExpression && initializer.getText().contains("declare"))
{
JSCallExpression declareCall = (JSCallExpression) variable.getInitializer();
JSCallExpression declareCall = (JSCallExpression) initializer;
declaration = declareCall;
break;
}
Expand Down
11 changes: 10 additions & 1 deletion src/com/chrisfolger/needsmoredojo/core/settings/DojoSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import javax.naming.NamingException;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
Expand Down Expand Up @@ -37,6 +36,7 @@ public class DojoSettings implements PersistentStateComponent<DojoSettings>
private boolean setupWarningDisabled;
private boolean refactoringEnabled;
private String supportedFileTypes;
private String importBlockNames;
private boolean singleQuotedModuleIDs;
// this will be used for converting to module specific sources later
private String version;
Expand All @@ -62,6 +62,7 @@ public DojoSettings()
addModuleIfThereAreNoneDefined = false;
allowCaseInsensitiveSearch = true;
supportedFileTypes = "jsp,js,php,html";
importBlockNames = "require,define";
singleQuotedModuleIDs = true;
}

Expand Down Expand Up @@ -158,6 +159,14 @@ public void setSupportedFileTypes(String supportedFileTypes) {
this.supportedFileTypes = supportedFileTypes;
}

public String getImportBlockNames() {
return importBlockNames;
}

public void setImportBlockNames(String importBlockNames) {
this.importBlockNames = importBlockNames;
}

@Nullable
@Override
public DojoSettings getState() {
Expand Down
2 changes: 1 addition & 1 deletion src/com/chrisfolger/needsmoredojo/core/util/JSUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class JSUtil
{
public static PsiElement addStatementBeforeElement(PsiElement parent, PsiElement element, String statement, String whitespace)
{
ASTNode node = JSChangeUtil.createStatementFromText(parent.getProject(), statement, JSUtils.getDialect(parent.getContainingFile()));
ASTNode node = JSChangeUtil.createExpressionFromText(parent.getProject(), statement, JSUtils.getDialect(parent.getContainingFile()));
parent.addBefore(node.getPsi(), element);

if(!whitespace.equals(""))
Expand Down
Loading