Browse Source

Editor context actions - explicitly setting EditorContext.Editor = null from ContextActionsRenderer when the ViewModel is discarded. Prevents leaks of reference to CodeEditorView when IContextActionProviders keep long-lived references to EditorContext. (thanks to Daniel!)

pull/15/head
mkonicek 15 years ago
parent
commit
2a826a7a5f
  1. 9
      src/AddIns/DisplayBindings/AvalonEdit.AddIn/Src/ContextActionsRenderer.cs
  2. 1
      src/AddIns/Misc/SharpRefactoring/Project/SharpRefactoring.csproj
  3. 4
      src/AddIns/Misc/SharpRefactoring/Project/Src/ContextActions/CacheClassAtCaret.cs
  4. 30
      src/AddIns/Misc/SharpRefactoring/Project/Src/ContextActions/DelegateAction.cs
  5. 19
      src/AddIns/Misc/SharpRefactoring/Project/Src/ContextActions/ParamCheck.cs
  6. 6
      src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/ContextAction.cs
  7. 8
      src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/ContextActionsService.cs
  8. 126
      src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/EditorContext.cs
  9. 8
      src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/IContextAction.cs
  10. 2
      src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/IContextActionCache.cs

9
src/AddIns/DisplayBindings/AvalonEdit.AddIn/Src/ContextActionsRenderer.cs

@ -108,9 +108,12 @@ namespace ICSharpCode.AvalonEdit.AddIn @@ -108,9 +108,12 @@ namespace ICSharpCode.AvalonEdit.AddIn
this.popup.OpenAtLineStart(this.Editor);
}
EditorActionsProvider lastActions;
ContextActionsBulbViewModel BuildPopupViewModel(ITextEditor editor)
{
var actionsProvider = ContextActionsService.Instance.GetAvailableActions(editor);
this.lastActions = actionsProvider;
return new ContextActionsBulbViewModel(actionsProvider);
}
@ -130,6 +133,11 @@ namespace ICSharpCode.AvalonEdit.AddIn @@ -130,6 +133,11 @@ namespace ICSharpCode.AvalonEdit.AddIn
this.popup.IsDropdownOpen = false;
this.popup.IsHiddenActionsExpanded = false;
this.popup.ViewModel = null;
if (this.lastActions != null) {
// Clear the context to prevent memory leaks in case some users kept long-lived references to EditorContext
this.lastActions.EditorContext.Clear();
this.lastActions = null;
}
}
void WorkbenchSingleton_Workbench_ViewClosed(object sender, ViewContentEventArgs e)
@ -139,6 +147,7 @@ namespace ICSharpCode.AvalonEdit.AddIn @@ -139,6 +147,7 @@ namespace ICSharpCode.AvalonEdit.AddIn
if (e.Content.PrimaryFileName == this.Editor.FileName) {
WorkbenchSingleton.Workbench.ViewClosed -= WorkbenchSingleton_Workbench_ViewClosed;
WorkbenchSingleton.Workbench.ActiveViewContentChanged -= WorkbenchSingleton_Workbench_ActiveViewContentChanged;
ClosePopup();
}
} catch {}
}

1
src/AddIns/Misc/SharpRefactoring/Project/SharpRefactoring.csproj

@ -77,7 +77,6 @@ @@ -77,7 +77,6 @@
<Compile Include="Src\ContextActions\CheckAssignmentCache.cs" />
<Compile Include="Src\ContextActions\CheckAssignmentNull.cs" />
<Compile Include="Src\ContextActions\CheckAssignmentNotNull.cs" />
<Compile Include="Src\ContextActions\DelegateAction.cs" />
<Compile Include="Src\ContextActions\Extensions.cs" />
<Compile Include="Src\ContextActions\FindTypeDeclarationsVisitor.cs" />
<Compile Include="Src\ContextActions\GenerateMember.cs" />

4
src/AddIns/Misc/SharpRefactoring/Project/Src/ContextActions/CacheClassAtCaret.cs

@ -12,7 +12,8 @@ using ICSharpCode.SharpDevelop.Refactoring; @@ -12,7 +12,8 @@ using ICSharpCode.SharpDevelop.Refactoring;
namespace SharpRefactoring.ContextActions
{
/// <summary>
/// Description of CacheClassAtCaret.
/// Caches information about the class at the caret in the editor.
/// Used by ContextActions.
/// </summary>
public class CacheClassAtCaret : IContextActionCache
{
@ -50,7 +51,6 @@ namespace SharpRefactoring.ContextActions @@ -50,7 +51,6 @@ namespace SharpRefactoring.ContextActions
return;
var c = this.Class;
// TODO cache
var classDecls = context.GetClassDeclarationsOnCurrentLine().ToList();
this.IsCaretAtClassDeclaration = classDecls.Count == 1 && (classDecls[0].FullyQualifiedName == c.FullyQualifiedName);

30
src/AddIns/Misc/SharpRefactoring/Project/Src/ContextActions/DelegateAction.cs

@ -1,30 +0,0 @@ @@ -1,30 +0,0 @@
// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt)
// This code is distributed under the GNU LGPL (for details please see \doc\license.txt)
using System;
using System.Collections.Generic;
using System.Linq;
using System.Windows;
using ICSharpCode.NRefactory;
using Ast = ICSharpCode.NRefactory.Ast;
using ICSharpCode.SharpDevelop;
using ICSharpCode.SharpDevelop.Dom;
using ICSharpCode.SharpDevelop.Dom.NRefactoryResolver;
using ICSharpCode.SharpDevelop.Editor;
using ICSharpCode.SharpDevelop.Editor.AvalonEdit;
using ICSharpCode.SharpDevelop.Refactoring;
namespace SharpRefactoring.ContextActions
{
public class DelegateAction : IContextAction
{
public string Title { get; set; }
public System.Action ExecuteAction { get; set; }
public void Execute()
{
if (this.ExecuteAction != null)
this.ExecuteAction();
}
}
}

19
src/AddIns/Misc/SharpRefactoring/Project/Src/ContextActions/ParamCheck.cs

@ -12,6 +12,15 @@ namespace SharpRefactoring.ContextActions @@ -12,6 +12,15 @@ namespace SharpRefactoring.ContextActions
/// </summary>
public abstract class ParamCheck : ContextAction
{
public override bool IsAvailable(EditorContext context)
{
var paramAtCaret = GetParameterAtCaret(context.CurrentSymbol);
if (paramAtCaret == null)
return false;
return IsAvailable(paramAtCaret.ResolvedType);
}
public LocalResolveResult GetParameterAtCaret(ResolveResult symbol)
{
LocalResolveResult param = symbol as LocalResolveResult;
@ -21,18 +30,10 @@ namespace SharpRefactoring.ContextActions @@ -21,18 +30,10 @@ namespace SharpRefactoring.ContextActions
return null;
if (!param.IsParameter)
return null;
// FIXME must be parameter definition, and method with body (not interface or abstract)
return param;
}
public override bool IsAvailable(EditorContext context)
{
var paramAtCaret = GetParameterAtCaret(context.CurrentSymbol);
if (paramAtCaret == null)
return false;
return IsAvailable(paramAtCaret.ResolvedType);
}
public override void Execute(EditorContext context)
{
var paramAtCaret = GetParameterAtCaret(context.CurrentSymbol);

6
src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/ContextAction.cs

@ -19,17 +19,19 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -19,17 +19,19 @@ namespace ICSharpCode.SharpDevelop.Refactoring
public abstract void Execute(EditorContext context);
public EditorContext Context { get; private set; }
public IEnumerable<IContextAction> GetAvailableActions(EditorContext context)
{
// re-initialize the context
this.Context = context;
if (this.IsAvailable(context))
yield return this;
}
public EditorContext Context { get; private set; }
public void Execute()
{
// context was re-initialized in GetAvailableActions
Execute(this.Context);
}
}

8
src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/ContextActionsService.cs

@ -54,12 +54,14 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -54,12 +54,14 @@ namespace ICSharpCode.SharpDevelop.Refactoring
}
}
/// <summary>
/// Returned by <see cref="ContextActionsService.GetAvailableActions()"></see>.
/// </summary>
public class EditorActionsProvider
{
ITextEditor editor { get; set; }
IList<IContextActionsProvider> providers { get; set; }
EditorContext context { get; set; }
public EditorContext EditorContext { get; set; }
public EditorActionsProvider(ITextEditor editor, IList<IContextActionsProvider> providers)
{
@ -70,7 +72,7 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -70,7 +72,7 @@ namespace ICSharpCode.SharpDevelop.Refactoring
this.editor = editor;
this.providers = providers;
ParserService.ParseCurrentViewContent();
this.context = new EditorContext(editor);
this.EditorContext = new EditorContext(editor);
}
public IEnumerable<IContextAction> GetVisibleActions()

126
src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/EditorContext.cs

@ -16,9 +16,9 @@ using ICSharpCode.SharpDevelop.Editor; @@ -16,9 +16,9 @@ using ICSharpCode.SharpDevelop.Editor;
namespace ICSharpCode.SharpDevelop.Refactoring
{
/// <summary>
/// Helper class for <see cref="IContextActionsProvider.GetAvailableActions"></see>.
/// Never keep long-lived references to this class
/// - the AST serves as one-time cache and does not get updated when editor text changes.
/// Contains information about code around the caret in the editor - useful for implementing Context actions.
/// Do not keep your own references to EditorContext.
/// It serves as one-time cache and does not get updated when editor text changes.
/// </summary>
public class EditorContext
{
@ -27,16 +27,16 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -27,16 +27,16 @@ namespace ICSharpCode.SharpDevelop.Refactoring
int CaretColumn { get; set; }
/// <summary>
/// Language independent.
/// The expression at editor caret. Language independent.
/// </summary>
public ExpressionResult CurrentExpression { get; private set; }
/// <summary>
/// Language independent.
/// The resolved symbol at editor caret. Language independent.
/// </summary>
public ResolveResult CurrentSymbol { get; private set; }
/// <summary>
/// Language independent.
/// ParseInformation for current file. Language independent.
/// </summary>
public ParseInformation CurrentParseInformation { get; private set; }
@ -49,22 +49,33 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -49,22 +49,33 @@ namespace ICSharpCode.SharpDevelop.Refactoring
}
}
/// <summary>
/// The editor line containing the caret.
/// </summary>
public IDocumentLine CurrentLine { get; private set; }
/// <summary>
/// Only available for C# and VB.
/// Parsed AST of the current editor line. Only available for C# and VB.
/// </summary>
public INode CurrentLineAST { get; private set; }
/// <summary>
/// Only available for C# and VB.
/// Parsed AST of the member containing the editor caret. Only available for C# and VB.
/// </summary>
public INode CurrentMemberAST { get; private set; }
/// <summary>
/// Only available for C# and VB.
/// Parsed AST of the element at editor caret. Only available for C# and VB.
/// </summary>
public INode CurrentElement { get; private set; }
INode CurrentElement { get; set; }
NRefactoryResolver Resolver { get; set; }
/// <summary>
/// Caches values shared by Context actions. Used in <see cref="GetCached"></see>
/// </summary>
Dictionary<Type, object> cachedValues = new Dictionary<Type, object>();
/// <summary>
/// Fully initializes the EditorContext.
/// </summary>
public EditorContext(ITextEditor editor)
{
if (editor == null)
@ -93,27 +104,43 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -93,27 +104,43 @@ namespace ICSharpCode.SharpDevelop.Refactoring
// DebugLog();
}
void DebugLog()
/// <summary>
/// Do not call from your Context actions - used by SharpDevelop.
/// Sets contents of editor context to null to prevent memory leaks. Used in case users implementing IContextActionProvider
/// keep long-lived references to EditorContext even when warned not to do so.
/// </summary>
public void Clear()
{
ICSharpCode.Core.LoggingService.Debug(string.Format(
@"
this.Editor = null;
this.CurrentElement = null;
this.CurrentLineAST = null;
this.CurrentMemberAST = null;
this.CurrentParseInformation = null;
this.CurrentSymbol = null;
this.Resolver = null;
this.cachedValues.Clear();
}
Context actions :
ExprAtCaret: {0}
----------------------
SymbolAtCaret: {1}
----------------------
CurrentLineAST: {2}
----------------------
AstNodeAtCaret: {3}
----------------------
CurrentMemberAST: {4}
----------------------",
CurrentExpression, CurrentSymbol, CurrentLineAST,
CurrentElement == null ? "" : CurrentElement.ToString().TakeStartEllipsis(400),
CurrentMemberAST == null ? "" : CurrentMemberAST.ToString().TakeStartEllipsis(400)));
/// <summary>
/// Gets cached value shared by context actions. Initializes a new value if not present.
/// </summary>
public T GetCached<T>() where T : IContextActionCache, new()
{
Type t = typeof(T);
if (cachedValues.ContainsKey(t)) {
return (T)cachedValues[t];
} else {
T cached = new T();
cached.Initialize(this);
cachedValues[t] = cached;
return cached;
}
}
/// <summary>
/// Checks if the caret is exactly at AST node of given type (e.g. "if" of IfElseStatement).
/// If yes, returns it. If not, returns null.
/// </summary>
public TNode GetCurrentElement<TNode>() where TNode : class, INode
{
if (this.CurrentElement is TNode)
@ -121,6 +148,10 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -121,6 +148,10 @@ namespace ICSharpCode.SharpDevelop.Refactoring
return null;
}
/// <summary>
/// Checks if caret is within AST node of given type (e.g. anywhere inside IfElseStatement).
/// If yes, returns it. If not, returns null.
/// </summary>
public TNode GetContainingElement<TNode>() where TNode : class, INode
{
var node = this.CurrentElement;
@ -133,22 +164,28 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -133,22 +164,28 @@ namespace ICSharpCode.SharpDevelop.Refactoring
return null;
}
Dictionary<Type, object> cachedValues = new Dictionary<Type, object>();
public T GetCached<T>() where T : IContextActionCache, new()
void DebugLog()
{
Type t = typeof(T);
if (cachedValues.ContainsKey(t)) {
return (T)cachedValues[t];
} else {
T cached = new T();
cached.Initialize(this);
cachedValues[t] = cached;
return cached;
}
ICSharpCode.Core.LoggingService.Debug(string.Format(
@"
Context actions :
ExprAtCaret: {0}
----------------------
SymbolAtCaret: {1}
----------------------
CurrentLineAST: {2}
----------------------
AstNodeAtCaret: {3}
----------------------
CurrentMemberAST: {4}
----------------------",
CurrentExpression, CurrentSymbol, CurrentLineAST,
CurrentElement == null ? "" : CurrentElement.ToString().TakeStartEllipsis(400),
CurrentMemberAST == null ? "" : CurrentMemberAST.ToString().TakeStartEllipsis(400)));
}
public static INode FindInnermostNode(INode node, Location position)
static INode FindInnermostNode(INode node, Location position)
{
if (node == null)
return null;
@ -214,12 +251,7 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -214,12 +251,7 @@ namespace ICSharpCode.SharpDevelop.Refactoring
var snippetParser = GetSnippetParser(editor);
if (snippetParser == null)
return null;
//try {
return snippetParser.Parse(currentLine.Text);
//}
//catch {
// return null;
//}
}
SnippetParser GetSnippetParser(ITextEditor editor)
@ -245,14 +277,10 @@ namespace ICSharpCode.SharpDevelop.Refactoring @@ -245,14 +277,10 @@ namespace ICSharpCode.SharpDevelop.Refactoring
INode GetCurrentMemberAST(ITextEditor editor)
{
//try {
var resolver = GetInitializedNRefactoryResolver(editor, this.CaretLine, this.CaretColumn);
if (resolver == null)
return null;
return resolver.ParseCurrentMember(editor.Document.Text);
//} catch {
// return null;
//}
}
NRefactoryResolver GetInitializedNRefactoryResolver(ITextEditor editor, int caretLine, int caretColumn)

8
src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/IContextAction.cs

@ -7,11 +7,17 @@ using System.Windows; @@ -7,11 +7,17 @@ using System.Windows;
namespace ICSharpCode.SharpDevelop.Refactoring
{
/// <summary>
/// Description of ContextAction.
/// One editor Context action.
/// </summary>
public interface IContextAction
{
/// <summary>
/// Title displayed in the context actions popup.
/// </summary>
string Title { get; }
/// <summary>
/// Executes this action. Called when this action is selected from the context actions popup.
/// </summary>
void Execute();
}
}

2
src/Main/Base/Project/Src/Services/RefactoringService/ContextActions/IContextActionCache.cs

@ -6,7 +6,7 @@ using System; @@ -6,7 +6,7 @@ using System;
namespace ICSharpCode.SharpDevelop.Refactoring
{
/// <summary>
/// Data shared by multiple Context actions. Stored in EditorContext.GetCached().
/// Temporary data shared by multiple Context actions. Stored in EditorContext.GetCached().
/// </summary>
public interface IContextActionCache
{

Loading…
Cancel
Save