From f7c529bc2ec7059e830b0117421406f04b33de82 Mon Sep 17 00:00:00 2001
From: Louis Orenstein
Date: Mon, 29 Apr 2013 16:09:05 -0500
Subject: Bug 319867 - Few issues in "Add ONLY Clause to USE Statement"
 refactoring

---
 .../refactoring/AddOnlyToUseStmtRefactoring.java   | 235 ++++++++++-----------
 .../infrastructure/FortranResourceRefactoring.java |  18 ++
 .../ui/refactoring/AddOnlyToUseStmtInputPage.java  |  88 ++++----
 3 files changed, 175 insertions(+), 166 deletions(-)

diff --git a/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/AddOnlyToUseStmtRefactoring.java b/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/AddOnlyToUseStmtRefactoring.java
index d6dcf657..a56ab64a 100644
--- a/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/AddOnlyToUseStmtRefactoring.java
+++ b/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/AddOnlyToUseStmtRefactoring.java
@@ -7,15 +7,18 @@
  *
  * Contributors:
  *    UIUC - Initial API and implementation
+ *    Louis Orenstein (Tech-X Corporation) - fix for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=319867
  *******************************************************************************/
 package org.eclipse.photran.internal.core.refactoring;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -29,16 +32,15 @@ import org.eclipse.ltk.core.refactoring.RefactoringStatus;
 import org.eclipse.ltk.core.refactoring.RefactoringStatusContext;
 import org.eclipse.photran.core.IFortranAST;
 import org.eclipse.photran.internal.core.analysis.binding.Definition;
+import org.eclipse.photran.internal.core.analysis.binding.ScopingNode;
 import org.eclipse.photran.internal.core.lexer.Terminal;
 import org.eclipse.photran.internal.core.lexer.Token;
 import org.eclipse.photran.internal.core.parser.ASTEntityDeclNode;
-import org.eclipse.photran.internal.core.parser.ASTListNode;
 import org.eclipse.photran.internal.core.parser.ASTModuleNode;
 import org.eclipse.photran.internal.core.parser.ASTSeparatedListNode;
 import org.eclipse.photran.internal.core.parser.ASTUseStmtNode;
 import org.eclipse.photran.internal.core.parser.GenericASTVisitor;
 import org.eclipse.photran.internal.core.refactoring.infrastructure.FortranEditorRefactoring;
-import org.eclipse.photran.internal.core.reindenter.Reindenter;
 import org.eclipse.photran.internal.core.vpg.PhotranTokenRef;
 import org.eclipse.photran.internal.core.vpg.PhotranVPG;
 
@@ -47,26 +49,26 @@ import org.eclipse.photran.internal.core.vpg.PhotranVPG;
  *
  * @author Kurt Hendle
  * @author Jeff Overbey - externalized strings
+ * @author Louis Orenstein (Tech-X Corporation) - fix for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=319867
  */
 public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
 {
     private String moduleName = null;
     private IProject projectInEditor = null;
-    private int numEntitiesInList = 0;
     private ASTUseStmtNode useNode = null;
     private List<IFile> filesContainingModule = null;
 
-    //private List<Definition> programEntities = new ArrayList<Definition>();
-    private ArrayList<String> entitiesInProgram = new ArrayList<String>();
+    private List<String> entitiesInProgram = new ArrayList<String>();
 
-    private List<Definition> moduleEntities = new ArrayList<Definition>();
-    private ArrayList<String> entitiesInModule = new ArrayList<String>();
+    private List<String> entitiesInModule = new ArrayList<String>();
+    private Map<String, Definition> moduleEntDefMap = new HashMap<String, Definition>();
 
     private List<Definition> existingOnlyList = new ArrayList<Definition>();
     private List<Definition> defsToAdd = new ArrayList<Definition>();
-    private HashMap<Integer, String> entitiesToAdd = new HashMap<Integer, String>();
+    private Set<String> entitiesToAdd = new HashSet<String>();
 
     private Set<PhotranTokenRef> allReferences = null;
+    private boolean onlyListInitiatlized = false;
 
     public AddOnlyToUseStmtRefactoring()
     {
@@ -77,57 +79,36 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
         initialize(file, selection);
     }
 
-    public ArrayList<String> getModuleEntityList()
+    public List<String> getModuleEntityList()
     {
         return entitiesInModule;
     }
 
     public void addToOnlyList(String name)
     {
-        if(!entitiesToAdd.containsValue(name))
-        {
-            entitiesToAdd.put(numEntitiesInList, PhotranVPG.canonicalizeIdentifier(name));
-
-            for(int i=0; i<moduleEntities.size(); i++)  //construct list of definitions
-            {
-                if(entitiesToAdd.get(numEntitiesInList).equals(moduleEntities.get(i).getCanonicalizedName()))
-                    defsToAdd.add(moduleEntities.get(i));
-            }
-
-            numEntitiesInList++;
+        final String id = PhotranVPG.canonicalizeIdentifier(name);
+        entitiesToAdd.add(id);
+        if (!defsToAdd.contains(moduleEntDefMap.get(id))) {
+            defsToAdd.add(moduleEntDefMap.get(id));
         }
     }
 
     public void removeFromOnlyList(String name)
     {
-        for(int i=0; i<entitiesToAdd.size(); i++)
-        {
-            if(name.equalsIgnoreCase(entitiesToAdd.get(i)))
-            {
-                for(int j=0; j<moduleEntities.size(); j++)  //remove def from list
-                {
-                    if(entitiesToAdd.get(i).equalsIgnoreCase(moduleEntities.get(j).getCanonicalizedName()))
-                    {
-                        defsToAdd.remove(moduleEntities.get(j));
-                        existingOnlyList.remove(moduleEntities.get(j));
-                    }
-                }
-
-                entitiesToAdd.remove(i);
-                numEntitiesInList--;
-                return;
-            }
-        }
+        entitiesToAdd.remove(name);
+        defsToAdd.remove(moduleEntDefMap.get(name));
     }
 
-    public HashMap<Integer, String> getNewOnlyList()
+    public Set<String> getNewOnlyList()
     {
         return entitiesToAdd;
     }
-
-    public int getNumEntitiesInModule()
+    
+    @Override
+    protected void preCheckFinalConditions(RefactoringStatus status, IProgressMonitor pm)
+        throws PreconditionFailure
     {
-        return moduleEntities.size();
+        // no-op
     }
 
     /* (non-Javadoc)
@@ -214,7 +195,7 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
 
         DeclarationVisitor visitor = new DeclarationVisitor();
         ast.accept(visitor);
-        //AST will be released later
+        vpg.releaseAST(this.fileInEditor);
     }
 
     private void getModuleDeclaredEntities() throws PreconditionFailure
@@ -232,44 +213,39 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
         if(moduleNode == null)
             fail(Messages.AddOnlyToUseStmtRefactoring_ModuleNodeNodeFound);
 
-        moduleEntities = moduleNode.getAllPublicDefinitions();
-        if(moduleEntities.isEmpty())
+        List<Definition> moduleDefs = moduleNode.getAllPublicDefinitions();        
+        if (moduleDefs.isEmpty()) {
             fail(Messages.AddOnlyToUseStmtRefactoring_NoDeclarationsInModule);
-        else
-        {
-            for(int i=0; i<moduleEntities.size(); i++)
-                entitiesInModule.add(moduleEntities.get(i).getCanonicalizedName());
+        } else {
+            for(Definition def : moduleDefs) {
+                moduleEntDefMap.put(def.getCanonicalizedName(), def);
+                entitiesInModule.add(def.getCanonicalizedName());
+            }
         }
     }
 
-    private void readExistingOnlyList()
+    public void readExistingOnlyList()
     {
         @SuppressWarnings("rawtypes")
         ASTSeparatedListNode existingOnlys = (ASTSeparatedListNode)useNode.getOnlyList();
         if(existingOnlys != null)
         {
-            for(int i=0; i<existingOnlys.size(); i++)
-            {
-                entitiesToAdd.put(i,
-                    PhotranVPG.canonicalizeIdentifier(existingOnlys.get(i).toString().trim()));
-            }
-
-            numEntitiesInList = entitiesToAdd.size();
-
-            for(int i=0; i<moduleEntities.size(); i++)  //construct list of definitions
-            {
-                if(entitiesToAdd.containsValue(moduleEntities.get(i).getCanonicalizedName()))
-                    existingOnlyList.add(moduleEntities.get(i));
+            for(Object existingOnly : existingOnlys) {
+                final String id = PhotranVPG.canonicalizeIdentifier(existingOnly.toString().trim());
+                entitiesToAdd.add(id);
+                existingOnlyList.add(moduleEntDefMap.get(id));
             }
         }
 
-        //FIXME add functionality to search file for existing uses of module vars
-        //and automatically make them be added to the list
-        IFortranAST ast = vpg.acquirePermanentAST(this.fileInEditor);
-        if(ast == null) return;
-
-        TokenVisitor visitor = new TokenVisitor();
-        ast.accept(visitor);
+        if (!onlyListInitiatlized) {
+            IFortranAST ast = vpg.acquirePermanentAST(fileInEditor);
+            if(ast == null) return;
+    
+            TokenVisitor visitor = new TokenVisitor();
+            ast.accept(visitor);
+            vpg.releaseAST(fileInEditor);
+            onlyListInitiatlized = true;
+        }
     }
 
     /* (non-Javadoc)
@@ -279,7 +255,18 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
     protected void doCreateChange(IProgressMonitor pm) throws CoreException,
         OperationCanceledException
     {
-        //nothing to do here
+        IFortranAST ast = vpg.acquirePermanentAST(this.fileInEditor);
+        if(ast == null) return;
+        
+        pm.subTask(Messages.AddOnlyToUseStmtRefactoring_InsertingUseStmt);
+        createAndInsertUseStmt(ast);
+  
+        pm.subTask(Messages.AddOnlyToUseStmtRefactoring_CreatingChangeObject);
+        addChangeFromModifiedAST(this.fileInEditor, pm);
+
+        vpg.releaseAST(this.fileInEditor);
+
+        pm.done();
     }
 
     /* (non-Javadoc)
@@ -299,19 +286,13 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
         if(ast == null) return;
 
         pm.subTask(Messages.AddOnlyToUseStmtRefactoring_CheckingForConflicts);
-        checkConflictingBindings(ast, pm, status);  //find conflicts
-
-        pm.subTask(Messages.AddOnlyToUseStmtRefactoring_InsertingUseStmt);
-        createAndInsertUseStmt(ast);
-
-        pm.subTask(Messages.AddOnlyToUseStmtRefactoring_CreatingChangeObject);
-        addChangeFromModifiedAST(fileInEditor, pm);
+        checkConflictingBindings(ast, pm, status); //find conflicts
+        
         vpg.releaseAST(fileInEditor);
 
         pm.done();
     }
-
-
+    
     /*
      * This method assumes that any existing only list is OK. Only checks for conflicting
      * bindings with NEW additions to only list.
@@ -319,39 +300,44 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
     private void checkConflictingBindings(IFortranAST ast, IProgressMonitor pm, RefactoringStatus status)
     {
         pm.subTask(Messages.AddOnlyToUseStmtRefactoring_FindingReferences);
-        allReferences = findModuleEntityRefs(ast);
-        //removeOriginalModuleRefs(); //possibly not needed - working without
 
-        for(Definition def : defsToAdd)
-        {
+        // find names that are the same as entities in the module that will be added to the "only" list
+        // find references to those names, and make sure those references don't change        
+        // find references to the current file and make sure that referenced names don't change
+        
+        for(String entityToAdd : entitiesToAdd) {
+            
+            final Set<PhotranTokenRef> newRefs = findModuleEntityRefs(ast, Arrays.asList(entityToAdd));
+            allReferences = newRefs;
+
+            Definition def = moduleEntDefMap.get(entityToAdd);
+            
             checkForConflictingBindings(pm,
                 new ConflictingBindingErrorHandler(status),
                 def,
+                (ScopingNode)ast.getRoot().getProgramUnitList().get(0),
                 allReferences,
                 def.getCanonicalizedName());
         }
     }
-
-  //Similar to ExtractProcedureRefactoring#localVariablesUsedIn
-    private Set<PhotranTokenRef> findModuleEntityRefs(IFortranAST ast)
+    
+    private Set<PhotranTokenRef> findModuleEntityRefs(IFortranAST ast, final Collection<String> defNames) 
     {
         final Set<PhotranTokenRef> result = new HashSet<PhotranTokenRef>();
-        final Collection<String> defNames = entitiesToAdd.values();
-        ast.accept(new GenericASTVisitor()
-        {
-            @Override public void visitToken(Token token)
-            {
-                if (token.getTerminal() == Terminal.T_IDENT)
-                {
-                    for (Definition def : token.resolveBinding())
-                    {
-                        if (defNames.contains(def.getCanonicalizedName()))
-                            result.addAll(def.findAllReferences(true));
+        
+        ast.accept(new GenericASTVisitor() {
+            @Override
+            public void visitToken(Token token) {
+                if (token.getTerminal() == Terminal.T_IDENT) {
+                    for (Definition def : token.resolveBinding()) {
+                        if (defNames.contains(def.getCanonicalizedName())) {
+                            result.add(token.getTokenRef());
+                        }
                     }
                 }
             }
         });
-
+        
         return result;
     }
 
@@ -380,9 +366,14 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
 
     private void createAndInsertUseStmt(IFortranAST ast)
     {
-      //create the new only selection
+    	// find the use node in the ast
+        Token useToken = ast.findTokenByStreamOffsetLength(
+        		useNode.getUseToken().getFileOffset(), useNode.getUseToken().getLength());
+        ASTUseStmtNode useToModify = (ASTUseStmtNode)useToken.getParent();
+        Collection<String> varNames = new TreeSet<String>(entitiesToAdd); // JO -- Sort names
+
+        //create the new only selection
         String newOnlyAdditions = " "; //$NON-NLS-1$
-        Collection<String> varNames = new TreeSet<String>(entitiesToAdd.values()); // JO -- Sort names
         Iterator<String> iter = varNames.iterator();
         int counter = 0;
 
@@ -397,17 +388,14 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
         //construct the new USE node and replace the old one in the ast
         ASTUseStmtNode newStmtNode;
         if(entitiesToAdd.size() > 0)// && entitiesToAdd.size() < moduleEntities.size())
-            newStmtNode = (ASTUseStmtNode)parseLiteralStatement("use " + //$NON-NLS-1$
-                useNode.getName().getText()+", only:" + newOnlyAdditions //$NON-NLS-1$
+            newStmtNode = (ASTUseStmtNode)parseLiteralStatement(useToken.getWhiteBefore() + "use " + //$NON-NLS-1$
+                useToModify.getName().getText()+", only:" + newOnlyAdditions //$NON-NLS-1$
                 + System.getProperty("line.separator")); //$NON-NLS-1$
         else
-            newStmtNode = (ASTUseStmtNode)parseLiteralStatement("use " + //$NON-NLS-1$
-                useNode.getName().getText() + System.getProperty("line.separator")); //$NON-NLS-1$
+            newStmtNode = (ASTUseStmtNode)parseLiteralStatement(useToken.getWhiteBefore() + "use " + //$NON-NLS-1$
+                useToModify.getName().getText() + System.getProperty("line.separator")); //$NON-NLS-1$
 
-        @SuppressWarnings("rawtypes")
-        ASTListNode body = (ASTListNode)useNode.getParent();
-        body.replaceChild(useNode, newStmtNode);
-        Reindenter.reindent(newStmtNode, ast);
+        useToModify.replaceWith(newStmtNode);
     }
 
     /* (non-Javadoc)
@@ -437,7 +425,7 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
         @Override public void visitToken(Token node)
         {
             String name = node.getText();
-            if(entitiesInModule.contains(name))
+            if(moduleEntDefMap.containsKey(name))
                 addToOnlyList(name);
         }
     }
@@ -457,11 +445,18 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
                 IFile file = conflict.tokenRef.getFile();
                 if(!filesContainingModule.contains(file) && file.getProject().equals(projectInEditor))
                 {
+                    Definition def = vpg.getDefinitionFor(conflict.tokenRef);
+                    Token token = def.getTokenRef().findToken();
                     String msg =
                         Messages.bind(
                             Messages.AddOnlyToUseStmtRefactoring_NameConflicts,
-                            conflict.name,
-                            vpg.getDefinitionFor(conflict.tokenRef));
+                            new Object[] {
+                                          conflict.name,
+                                          def.getCanonicalizedName(),
+                                          def.getClassification(),
+                                          token.getLine(),
+                                          conflict.tokenRef.getFilename()
+                            });
                     RefactoringStatusContext context = createContext(conflict.tokenRef); // Highlights problematic definition
                     status.addError(msg, context);
                 }
@@ -492,15 +487,15 @@ public class AddOnlyToUseStmtRefactoring extends FortranEditorRefactoring
             if(entitiesInProgram.contains(newName))
             {
                 // The entity with the new name will shadow the definition to which this binding resolves
-                status.addError(
-                    Messages.bind(
-                        Messages.AddOnlyToUseStmtRefactoring_AddingWouldChangeMeaningOf,
-                        new Object[] {
-                            newName,
-                            reference.getText(),
-                            reference.getLine(),
-                            reference.getTokenRef().getFilename() }),
-                    createContext(reference)); // Highlight problematic reference
+                String msg = Messages.bind(
+                    Messages.AddOnlyToUseStmtRefactoring_AddingWouldChangeMeaningOf,
+                    new Object[] { newName,
+                                   reference.getText(),
+                                   reference.getLine(),
+                                   reference.getTokenRef().getFilename() 
+                    });
+                RefactoringStatusContext context = createContext(reference); // Highlight problematic reference
+                status.addError(msg, context);
             }
         }
     }
diff --git a/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/infrastructure/FortranResourceRefactoring.java b/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/infrastructure/FortranResourceRefactoring.java
index 90d2e084..aa69af2d 100644
--- a/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/infrastructure/FortranResourceRefactoring.java
+++ b/org.eclipse.photran.core.vpg/src/org/eclipse/photran/internal/core/refactoring/infrastructure/FortranResourceRefactoring.java
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *    UIUC - Initial API and implementation
+ *    Louis Orenstein (Tech-X Corporation) - fix for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=319867
  *******************************************************************************/
 package org.eclipse.photran.internal.core.refactoring.infrastructure;
 
@@ -69,6 +70,7 @@ import org.eclipse.rephraserengine.core.vpg.refactoring.VPGResourceRefactoring;
 /**
  * This is a base class for all Photran refactorings that apply to multiple files
  * @author Jeff Overbey, Timofey Yuvashev
+ * @author Louis Orenstein (Tech-X Corporation) - fix for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=319867
  */
 public abstract class FortranResourceRefactoring
     extends VPGResourceRefactoring<IFortranAST, Token, PhotranVPG>
@@ -773,6 +775,19 @@ public abstract class FortranResourceRefactoring
     {
         checkForConflictingBindings(pm, callback, definitionToCheck, allReferences, Arrays.asList(newNames));
     }
+    
+    protected static void checkForConflictingBindings(
+        IProgressMonitor pm,
+        IConflictingBindingCallback callback,
+        Definition definitionToCheck,
+        ScopingNode scopeToCheck,
+        Collection<PhotranTokenRef> allReferences,
+        String... newNames)
+    {
+        CheckForConflictBindings check = new CheckForConflictBindings(definitionToCheck, allReferences, Arrays.asList(newNames));
+        check.scopeOfDefinitionToCheck = scopeToCheck;
+        check.check(pm, callback);
+    }
 
     /**
      * Given a {@link Definition} and a list of references to that Definition
@@ -1003,6 +1018,9 @@ public abstract class FortranResourceRefactoring
                     pm.subTask(Messages.bind(Messages.FortranResourceRefactoring_CheckingForReferencesTo, newName, importingScope.describe()));
                     shadowedDefinitions.addAll(importingScope.manuallyResolve(token));
                 }
+                if (definitionToCheck != null) {
+                    shadowedDefinitions.remove(definitionToCheck.getTokenRef());
+                }
 
                 for (PhotranTokenRef def : shadowedDefinitions)
                 {
diff --git a/org.eclipse.photran.ui.vpg/src/org/eclipse/photran/internal/ui/refactoring/AddOnlyToUseStmtInputPage.java b/org.eclipse.photran.ui.vpg/src/org/eclipse/photran/internal/ui/refactoring/AddOnlyToUseStmtInputPage.java
index 8a3b8b51..62be5396 100644
--- a/org.eclipse.photran.ui.vpg/src/org/eclipse/photran/internal/ui/refactoring/AddOnlyToUseStmtInputPage.java
+++ b/org.eclipse.photran.ui.vpg/src/org/eclipse/photran/internal/ui/refactoring/AddOnlyToUseStmtInputPage.java
@@ -7,17 +7,19 @@
  *
  * Contributors:
  *    UIUC - Initial API and implementation
+ *    Louis Orenstein (Tech-X Corporation) - fix for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=319867
  *******************************************************************************/
 package org.eclipse.photran.internal.ui.refactoring;
 
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.List;
+import java.util.Set;
 
+import org.eclipse.jface.wizard.IWizardPage;
 import org.eclipse.photran.internal.core.refactoring.AddOnlyToUseStmtRefactoring;
 import org.eclipse.rephraserengine.ui.refactoring.CustomUserInputPage;
 import org.eclipse.swt.SWT;
-import org.eclipse.swt.events.SelectionEvent;
-import org.eclipse.swt.events.SelectionListener;
+import org.eclipse.swt.custom.ScrolledComposite;
 import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
@@ -26,71 +28,65 @@ import org.eclipse.swt.widgets.Label;
 /**
  *
  * @author Kurt Hendle
+ * @author Louis Orenstein (Tech-X Corporation) - fix for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=319867
  */
 public class AddOnlyToUseStmtInputPage extends CustomUserInputPage<AddOnlyToUseStmtRefactoring>
 {
-    protected ArrayList<String> entities;
-    protected HashMap<Integer, String> newOnlyList;
-    protected ArrayList<Button> checkList;
-    protected Button check;
+    protected List<Button> checkList;
+
+    @Override
+    public IWizardPage getNextPage()
+    {
+        for(Button checkBox : checkList) {
+            boolean isChecked = checkBox.getSelection();
+            final String buttonText = checkBox.getText();
+            if (isChecked) {
+                getRefactoring().addToOnlyList(buttonText);
+            } else {
+                getRefactoring().removeFromOnlyList(buttonText);
+            }
+        }
+        
+        return super.getNextPage();
+    }
 
     @Override public void createControl(Composite parent)
     {
-        entities = getRefactoring().getModuleEntityList();
-        newOnlyList = getRefactoring().getNewOnlyList();
+        getRefactoring().readExistingOnlyList();
+        List<String> entities = getRefactoring().getModuleEntityList();
+        Set<String> newOnlyList = getRefactoring().getNewOnlyList();
         checkList = new ArrayList<Button>();
 
-        Composite top = new Composite(parent, SWT.NONE);
-        initializeDialogUnits(top);
-        setControl(top);
-
-        top.setLayout(new GridLayout(1,false));
+        // using a ScrolledComposite so the list is scrollable
+        ScrolledComposite container = new ScrolledComposite(parent, SWT.H_SCROLL | SWT.V_SCROLL);
+        initializeDialogUnits(container);
+        setControl(container);
+        
+        Composite top = new Composite(container, SWT.NONE);
+        top.setLayout(new GridLayout(1, false));
         Composite group = top;
 
         Label lbl = new Label(group, SWT.NONE);
         lbl.setText(Messages.AddOnlyToUseStmtInputPage_SelectModuleEntitiesLabel);
 
-        for(int i=0; i<getRefactoring().getNumEntitiesInModule(); i++)
+        for(String entityName : entities) 
         {
-            check = new Button(group, SWT.CHECK);
-            check.setText(entities.get(i));
+            Button check = new Button(group, SWT.CHECK);
+            check.setText(entityName);
 
-            if(newOnlyList.containsValue(entities.get(i))){
+            if(newOnlyList.contains(entityName)){
                 check.setSelection(true);
-                //getRefactoring().addToOnlyList(i, entities.get(i));
             }
 
             checkList.add(check);
         }
 
-        //turns out need to add listeners last to make this work correctly
-        for(int i=0; i<getRefactoring().getNumEntitiesInModule(); i++)
-        {
-            final int index = i;
-            checkList.get(i).addSelectionListener(new SelectionListener()
-            {
-                public void widgetDefaultSelected(SelectionEvent e)
-                {
-                    widgetSelected(e);
-                }
-
-                public void widgetSelected(SelectionEvent e)
-                {
-                    boolean isChecked = checkList.get(index).getSelection();
-
-                    if(isChecked)
-                    {
-                        getRefactoring().addToOnlyList(entities.get(index));
-                    }
-                    else //if(!isChecked)
-                    {
-                        getRefactoring().removeFromOnlyList(entities.get(index));
-                    }
-                }
-            });
-        }
-
         Label instruct = new Label(top, SWT.NONE);
         instruct.setText(Messages.AddOnlyToUseStmtInputPage_ClickOKMessage);
+        
+        container.setExpandHorizontal(true);
+        container.setExpandVertical(true);
+        container.setContent(top);
+        container.setMinSize(top.computeSize(SWT.DEFAULT, SWT.DEFAULT));
     }
 }
-- 
cgit v1.2.3