Skip to content

Commit d7ccb02

Browse files
tommy9tommy9
committed
Start on error reporting and some refactoring
1 parent 26bd6f2 commit d7ccb02

16 files changed

+149
-581
lines changed

‎Rubberduck.Core/UI/Command/Refactorings/Notifiers/ExtractMethodFailedNotifier.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ protected override string Message(RefactoringException exception)
1919
{
2020
case UnableToMoveVariableDeclarationException unableToMoveVariableDeclaration:
2121
Logger.Warn(unableToMoveVariableDeclaration);
22-
return RefactoringsUI.ExtractMethod_InvalidSelectionMessage; //TODO - improve this message
22+
return RefactoringsUI.ExtractMethod_UnableToMoveVariableDeclarationMessage; //TODO - improve this message to show declaration
23+
case InvalidTargetSelectionException invalidTargetSelection:
24+
return string.Format(RefactoringsUI.ExtractMethod_InvalidSelectionMessage, invalidTargetSelection.Message);
25+
//case
2326
default:
2427
return base.Message(exception);
2528
}

‎Rubberduck.Core/UI/Refactorings/ExtractMethod/ExtractMethodPresenter.cs‎

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace Rubberduck.Refactorings.ExtractMethod
77
{
88
internal class ExtractMethodPresenter : RefactoringPresenterBase<ExtractMethodModel>, IExtractMethodPresenter
99
{
10-
private static readonly DialogData DialogData = DialogData.Create(RefactoringsUI.ExtractMethod_Caption, 600, 600); //TODO - get appropriate size
10+
private static readonly DialogData DialogData = DialogData.Create(RefactoringsUI.ExtractMethod_Caption, 400, 800); //TODO - get appropriate size
1111

1212
private readonly IMessageBox _messageBox;
1313

@@ -25,50 +25,5 @@ override public ExtractMethodModel Show()
2525
//TODO - test not cancelled or other invalid output?
2626
return base.Show();
2727
}
28-
29-
//ExtractMethodModel IExtractMethodPresenter.Show(IExtractMethodModel methodModel, IExtractMethodProc extractMethodProc)
30-
//{
31-
// throw new System.NotImplementedException();
32-
//}
33-
/*
34-
private void PrepareView(ExtractMethodModel extractedMethodModel)
35-
{
36-
_view.ViewModel.OldMethodName = extractedMethodModel.SourceMember.IdentifierName;
37-
_view.ViewModel.MethodName = extractedMethodModel.SourceMember.IdentifierName;
38-
_view.ViewModel.Inputs = extractedMethodModel.Inputs;
39-
_view.ViewModel.Outputs = extractedMethodModel.Outputs;
40-
_view.ViewModel.Locals =
41-
extractedMethodModel.Locals.Select(
42-
variable =>
43-
new ExtractedParameter(variable.AsTypeName, PassedBy.ByVal, variable.IdentifierName))
44-
.ToList();
45-
46-
var returnValues = new[] {new ExtractedParameter(string.Empty, PassedBy.ByVal)}
47-
.Union(_view.ViewModel.Outputs)
48-
.Union(_view.ViewModel.Inputs)
49-
.ToList();
50-
51-
_view.ViewModel.ReturnValues = returnValues;
52-
53-
//_view.RefreshPreview += (object sender, EventArgs e) => { GeneratePreview(extractedMethodModel, extractMethodProc); };
54-
55-
//_view.OnRefreshPreview();
56-
}
57-
58-
private void GeneratePreview(IExtractMethodModel extractMethodModel,IExtractMethodProc extractMethodProc )
59-
{
60-
extractMethodModel.Method.MethodName = _view.ViewModel.MethodName;
61-
extractMethodModel.Method.Accessibility = _view.ViewModel.Accessibility;
62-
extractMethodModel.Method.Parameters = _view.ViewModel.Parameters;
63-
//
64-
// extractMethodModel.Method.ReturnValue = _view.ReturnValue;
65-
// extractMethodModel.Method.SetReturnValue = _view.SetReturnValue;
66-
//
67-
var extractedMethod = extractMethodProc.createProc(extractMethodModel);
68-
var code = extractedMethod.Split(new[]{Environment.NewLine}, StringSplitOptions.RemoveEmptyEntries);
69-
code = _indenter.Indent(code).ToArray();
70-
_view.ViewModel.Preview = string.Join(Environment.NewLine, code);
71-
}
72-
*/
7328
}
7429
}

‎Rubberduck.Core/UI/Refactorings/ExtractMethod/ExtractMethodViewModel.cs‎

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,12 @@
22
using System.Collections.Generic;
33
using System.Collections.ObjectModel;
44
using System.Linq;
5-
using System.Windows.Forms;
6-
using NLog;
75
using Rubberduck.Interaction;
86
using Rubberduck.Parsing.Grammar;
97
using Rubberduck.Parsing.Symbols;
108
using Rubberduck.Parsing.VBA;
119
using Rubberduck.Refactorings;
1210
using Rubberduck.Refactorings.ExtractMethod;
13-
using Rubberduck.UI.Command;
1411

1512
namespace Rubberduck.UI.Refactorings.ExtractMethod
1613
{
@@ -25,7 +22,7 @@ public ExtractMethodViewModel(RubberduckParserState state, ExtractMethodModel mo
2522
{
2623
State = state;
2724
_messageBox = messageBox;
28-
_model = model;
25+
//_model = model;
2926
}
3027

3128
private bool _wired;
@@ -121,25 +118,5 @@ protected override void DialogOk()
121118
{
122119
base.DialogOk();
123120
}
124-
125-
private ExtractMethodModel _model;
126-
//public ExtractMethodModel Model
127-
//{
128-
// get => _model;
129-
// set
130-
// {
131-
// _model = value;
132-
// // Note: some of the property change will cascade
133-
// // so we do not need to call all possible properties
134-
// // depending on the Model.
135-
// OnPropertyChanged(nameof(NewMethodName));
136-
// OnPropertyChanged(nameof(ReturnParameter));
137-
// OnPropertyChanged(nameof(Parameters));
138-
// if (!_wired)
139-
// {
140-
// WireParameterEvents();
141-
// }
142-
// }
143-
//}
144121
}
145122
}

‎Rubberduck.Refactorings/Exceptions/InvalidTargetSelectionException.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ public InvalidTargetSelectionException(QualifiedSelection targetSelection)
99
TargetSelection = targetSelection;
1010
}
1111

12+
public InvalidTargetSelectionException(QualifiedSelection targetSelection, string message)
13+
{
14+
TargetSelection = targetSelection;
15+
Message = message;
16+
}
17+
1218
public QualifiedSelection TargetSelection { get; }
19+
public override string Message { get; }
1320
}
1421
}

‎Rubberduck.Refactorings/ExtractMethod/ExtractMethodModel.cs‎

Lines changed: 44 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
using System.Collections.ObjectModel;
44
using System.Linq;
55
using Antlr4.Runtime;
6+
using Rubberduck.Parsing;
67
using Rubberduck.Parsing.Grammar;
78
using Rubberduck.Parsing.Symbols;
8-
using Rubberduck.VBEditor;
99
using Rubberduck.Parsing.VBA;
1010
using Rubberduck.SmartIndenter;
11+
using Rubberduck.VBEditor;
1112
using Rubberduck.VBEditor.Extensions;
13+
using Rubberduck.Refactorings.Exceptions;
1214
using Rubberduck.Refactorings.Exceptions.ExtractMethod;
13-
using Rubberduck.Parsing;
14-
using System.Text.RegularExpressions;
1515

1616
namespace Rubberduck.Refactorings.ExtractMethod
1717
{
@@ -28,9 +28,8 @@ public class ExtractMethodModel : IRefactoringModel
2828
public IEnumerable<string> ComponentNames =>
2929
_declarationFinderProvider.DeclarationFinder.UserDeclarations(DeclarationType.Member).Where(d => d.ComponentName == QualifiedSelection.QualifiedName.ComponentName)
3030
.Select(d => d.IdentifierName);
31-
public string SourceMethodName { get; private set; }
31+
public string SourceMethodName { get => TargetMethod.IdentifierName; }
3232
public Declaration TargetMethod { get; set; }
33-
//public IEnumerable<Declaration> SourceVariables { get; private set; }
3433
public string NewMethodName
3534
{
3635
get => extractedMethod.MethodName;
@@ -61,47 +60,27 @@ public ExtractMethodModel(IDeclarationFinderProvider declarationFinderProvider,
6160

6261
private void Setup()
6362
{
64-
//if (string.IsNullOrWhiteSpace(NewMethodName))
65-
//{
66-
// NewMethodName = RefactoringsUI.ExtractMethod_DefaultNewMethodName; //Check for conflicts - see other document for example code
67-
//}
63+
var functionReturnValueAssignments = TargetMethod.References
64+
.Where(r => QualifiedSelection.Selection.Contains(r.Selection) &&
65+
(r.IsAssignment || r.IsSetAssignment));
66+
if (functionReturnValueAssignments.Count() != 0)
67+
{
68+
var firstSelection = functionReturnValueAssignments.FirstOrDefault().QualifiedSelection;
69+
var message = "Selection modifies the return value of the enclosing function"; //TODO - get from resx
70+
throw new InvalidTargetSelectionException(firstSelection, message);
71+
}
6872

6973
SelectedCode = string.Join(Environment.NewLine, SelectedContexts.Select(c => c.GetText()));
7074

71-
//SourceVariables = _declarationFinderProvider.DeclarationFinder.UserDeclarations(DeclarationType.Variable)
72-
// .Where(d => (Selection.Selection.Contains(d.Selection) &&
73-
// d.QualifiedName.QualifiedModuleName == Selection.QualifiedName) ||
74-
// d.References.Any(r =>
75-
// r.QualifiedModuleName.ComponentName == Selection.QualifiedName.ComponentName
76-
// && r.QualifiedModuleName.ComponentName ==
77-
// d.QualifiedName.QualifiedModuleName.ComponentName
78-
// && Selection.Selection.Contains(r.Selection)))
79-
// .OrderBy(d => d.Selection.StartLine)
80-
// .ThenBy(d => d.Selection.StartColumn);
81-
82-
var declarationsInSelection = _declarationFinderProvider.DeclarationFinder.UserDeclarations(DeclarationType.Variable)
83-
.Where(d => QualifiedSelection.Selection.Contains(d.Selection) &&
84-
d.QualifiedName.QualifiedModuleName == QualifiedSelection.QualifiedName)
85-
.OrderBy(d => d.Selection.StartLine)
86-
.ThenBy(d => d.Selection.StartColumn);
87-
var referencesOfDeclarationsInSelection = (from dec in declarationsInSelection select dec.References).ToArray(); //debug purposes
75+
var declarationsInSelection = GetDeclarationsInSelection(QualifiedSelection);
8876

8977
var sourceMethodParameters = ((IParameterizedDeclaration)TargetMethod).Parameters;
9078
var sourceMethodSelection = new QualifiedSelection(QualifiedSelection.QualifiedName,
9179
new Selection(TargetMethod.Context.Start.Line, TargetMethod.Context.Start.Column, TargetMethod.Context.Stop.Line, TargetMethod.Context.Stop.Column));
9280

93-
//TODO - refactor below and declarationsInSelection to generic declarations in selection method if stay consistent
94-
var declarationsInParentMethod = _declarationFinderProvider.DeclarationFinder.UserDeclarations(DeclarationType.Variable)
95-
.Where(d => sourceMethodSelection.Selection.Contains(d.Selection) &&
96-
d.QualifiedName.QualifiedModuleName == sourceMethodSelection.QualifiedName)
97-
.OrderBy(d => d.Selection.StartLine)
98-
.ThenBy(d => d.Selection.StartColumn);
81+
var declarationsInParentMethod = GetDeclarationsInSelection(sourceMethodSelection);
9982

10083
//List of "inbound" variables. Parent procedure parameters + explicit dims which get referenced inside the selection.
101-
//Ideally excluding those declared but not assigned before the selection. Refinement to change this later
102-
//No need to check if reference is outside of method because just dealing with local parameters and declarations
103-
//Add function reference if it is a function?
104-
//Would ideally identify variables assigned before the selection, not just declared. Could assume any reference before the selection is an assignment?
10584
//TODO - add case where reference earlier in the same line as where the selection starts (unusual but could exist if using colons to separate multiple statements)
10685
var inboundParameters = sourceMethodParameters.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)));
10786
var inboundLocalVariables = declarationsInParentMethod.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
@@ -112,7 +91,33 @@ private void Setup()
11291
var outboundVariables = sourceMethodParameters.Concat(declarationsInParentMethod)
11392
.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) && d.References.Any(r => r.Selection.StartLine > QualifiedSelection.Selection.EndLine));
11493

115-
//Set up parameters
94+
SetUpParameters(inboundVariables, outboundVariables);
95+
96+
//Variables to have declarations moved out of the selection
97+
// - where declaration is in the selection and it is a ByRef variable i.e. intersection of declarations in selection and outbound
98+
_declarationsToMoveOut = declarationsInSelection.Intersect(outboundVariables)
99+
.OrderByDescending(d => d.Selection.StartLine)
100+
.ThenByDescending(d => d.Selection.StartColumn);
101+
102+
//Variables to have declarations moved into the selection
103+
// - where declaration is before the selection but only references are inside the selection
104+
_declarationsToMoveIn = declarationsInParentMethod.Except(declarationsInSelection)
105+
.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
106+
!d.References.Any(r => r.Selection.EndLine < QualifiedSelection.Selection.StartLine) &&
107+
!d.References.Any(r => r.Selection.StartLine > QualifiedSelection.Selection.EndLine));
108+
}
109+
110+
private IOrderedEnumerable<Declaration> GetDeclarationsInSelection(QualifiedSelection qualifiedSelection)
111+
{
112+
return _declarationFinderProvider.DeclarationFinder.UserDeclarations(DeclarationType.Variable)
113+
.Where(d => qualifiedSelection.Selection.Contains(d.Selection) &&
114+
d.QualifiedName.QualifiedModuleName == qualifiedSelection.QualifiedName)
115+
.OrderBy(d => d.Selection.StartLine)
116+
.ThenBy(d => d.Selection.StartColumn);
117+
}
118+
119+
private void SetUpParameters(IEnumerable<Declaration> inboundVariables, IEnumerable<Declaration> outboundVariables)
120+
{
116121
Parameters = new ObservableCollection<ExtractMethodParameter>();
117122

118123
foreach (var declaration in inboundVariables.Union(outboundVariables))
@@ -149,24 +154,6 @@ private void Setup()
149154
paramType, declaration.IdentifierName,
150155
declaration.IsArray, declaration.IsObject, canReturn));
151156
}
152-
153-
//Variables to have declarations moved out of the selection
154-
// - where declaration is in the selection and it is a ByRef variable i.e. intersection of declarations in selection and outbound
155-
_declarationsToMoveOut = declarationsInSelection.Intersect(outboundVariables)
156-
.OrderByDescending(d => d.Selection.StartLine)
157-
.ThenByDescending(d => d.Selection.StartColumn);
158-
159-
//Variables to have declarations moved into the selection
160-
// - where declaration is before the selection but only references are inside the selection
161-
_declarationsToMoveIn = declarationsInParentMethod.Except(declarationsInSelection)
162-
.Where(d => d.References.Any(r => QualifiedSelection.Selection.Contains(r.Selection)) &&
163-
!d.References.Any(r => r.Selection.EndLine < QualifiedSelection.Selection.StartLine) &&
164-
!d.References.Any(r => r.Selection.StartLine > QualifiedSelection.Selection.EndLine));
165-
166-
//List of neither "inbound" or "outbound" (need the declaration copied inside the selection OR moved if careful but can leave inspections to pick up unnecessary declarations)
167-
//Only applies if the list of inbound variables excludes those declared but not assigned before the selection
168-
//????
169-
170157
}
171158

172159
public string SelectedCode { get; private set; }
@@ -213,7 +200,7 @@ public string SelectedCodeToExtract
213200
//1) Go up to Block ancester
214201
//2) Find BlockStmt enclosing the declaration
215202
//3) Check if preceding or following BlockStmts (if exist) are on the same line (preceding.Stop or following.Start)
216-
//4) Throw error or rebuild line
203+
//4) preceding blocks seem okay except for indentation. following blocks
217204

218205

219206
//Remove declaration range from selected code
@@ -333,6 +320,7 @@ public string NewMethodCode
333320

334321
private string FrontPadding(VBAParser.BlockStmtContext context)
335322
{
323+
//TODO - starts from block statement but that could be following another statement e.g. a = 1 : b = 2, so better to find whole line
336324
var paddingChars = context.Start.Column;
337325
if (paddingChars > 0)
338326
{

‎Rubberduck.Refactorings/ExtractMethod/ExtractMethodRefactoring.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ protected override ExtractMethodModel InitializeModel(Declaration target)
6363
}
6464
else
6565
{
66-
throw new InvalidTargetSelectionException(_selectionProvider.ActiveSelection().GetValueOrDefault());
66+
throw new InvalidTargetSelectionException(_selectionProvider.ActiveSelection().GetValueOrDefault(),
67+
Validator.InvalidContexts.FirstOrDefault().Item2);
6768
}
6869
}
6970

‎Rubberduck.Refactorings/ExtractMethod/IExtractMethodRule.cs‎

Lines changed: 0 additions & 83 deletions
This file was deleted.

0 commit comments

Comments
 (0)