Fixed suggestions by FxCop for NavigationService;

git-svn-id: svn://svn.sharpdevelop.net/sharpdevelop/trunk@1577 1ccf3a8d-04fe-1044-b7c0-cef0b8235c61

src/Main/Base/Project/Src/Commands/MenuItemBuilders.cs

@@ -25,16 +25,17 @@ namespace ICSharpCode.SharpDevelop.Commands
 	{
 		// TODO: refactor BuildSubmenu to add a choice between flat and perfile, eventually per class/method sorting of the list
 		
-		ToolStripItem[] BuildMenuFlat(List<INavigationPoint> points, int additionalItems)
+		ToolStripItem[] BuildMenuFlat(ICollection<INavigationPoint> points, int additionalItems)
 		{
 			ToolStripItem[] items = new ToolStripItem[points.Count+additionalItems];
 			MenuCommand cmd = null;
 			INavigationPoint p = null;
+			List<INavigationPoint> list = new List<INavigationPoint>(points);
 			
 			int n = points.Count-1; // the last point
 			int i = 0;
 			while (i<points.Count) {
-				p = points[n-i];
+				p = list[n-i];
 				cmd = new MenuCommand(p.Description, new EventHandler(NavigateTo));
 				cmd.Tag = p;
 //					if (p == NavigationService.CurrentPosition) {
@@ -44,7 +45,7 @@ namespace ICSharpCode.SharpDevelop.Commands
 			}
 			return items;
 		}
-		ToolStripItem[] BuildMenuByFile(List<INavigationPoint> points, int additionalItems)
+		ToolStripItem[] BuildMenuByFile(ICollection<INavigationPoint> points, int additionalItems)
 		{
 			Dictionary<string, List<INavigationPoint>> files =
 				new Dictionary<string, List<INavigationPoint>>();
@@ -107,7 +108,7 @@ namespace ICSharpCode.SharpDevelop.Commands
 		{
 			MenuCommand cmd = null;
 			if (NavigationService.CanNavigateBack || NavigationService.CanNavigateForwards) {
-				List<INavigationPoint> points = NavigationService.GetListOfPoints();
+				ICollection<INavigationPoint> points = NavigationService.Points;
 
 				//ToolStripItem[] items = BuildMenuFlat(points, numberOfAdditionalItems);
 				ToolStripItem[] items = BuildMenuByFile(points, numberOfAdditionalItems);

src/Main/Base/Project/Src/Services/NavigationService/DefaultNavigationPoint.cs

@@ -6,7 +6,7 @@
 // </file>
 
 using System;
-using System.Drawing;
+using System.Globalization;
 
 namespace ICSharpCode.Core
 {
@@ -24,7 +24,7 @@ namespace ICSharpCode.Core
 		public DefaultNavigationPoint(string fileName) : this(fileName, null) {}
 		public DefaultNavigationPoint(string fileName, object data)
 		{
-			this.fileName = fileName;
+			this.fileName = fileName == null ? String.Empty : fileName;
 			this.data = data;
 		}
 		#endregion
@@ -32,7 +32,8 @@ namespace ICSharpCode.Core
 		#region overrides
 		public override string ToString()
 		{
-			return String.Format("[{0}: {1}]",
+			return String.Format(CultureInfo.CurrentCulture,
+			                     "[{0}: {1}]",
 			                     this.GetType().Name,
 			                     this.Description);
 		}
@@ -47,7 +48,8 @@ namespace ICSharpCode.Core
 		
 		public virtual string Description {
 			get {
-				return String.Format("{0}: {1}", fileName, data);
+				return String.Format(CultureInfo.CurrentCulture,
+				                     "{0}: {1}", fileName, data);
 			}
 		}
 		
@@ -82,7 +84,7 @@ namespace ICSharpCode.Core
 				data = value;
 			}
 		}
-				
+		
 		public virtual void JumpTo()
 		{
 			FileService.JumpToFilePosition(this.FileName, 0, 0);
@@ -90,7 +92,7 @@ namespace ICSharpCode.Core
 		
 		public void FileNameChanged(string newName)
 		{
-			fileName = newName;
+			fileName = newName == null ? String.Empty : newName;
 		}
 		
 		public virtual void ContentChanging(object sender, EventArgs e)
@@ -99,32 +101,53 @@ namespace ICSharpCode.Core
 		}
 		#endregion
 
+		#region Equality
+
+		public override bool Equals(object obj)
+		{
+			DefaultNavigationPoint b = obj as DefaultNavigationPoint;
+			if (b == null) return false;
+			return this.FileName == b.FileName;
+		}
+		
+		public override int GetHashCode()
+		{
+			return this.FileName.GetHashCode();
+		}
+		#endregion
+
 		#region IComparable
 		public virtual int CompareTo(object obj)
 		{
+			if (obj == null) return 1;
 			if (this.GetType() != obj.GetType()) {
+				// if of different types, sort the types by name
 				return this.GetType().Name.CompareTo(obj.GetType().Name);
 			}
 			DefaultNavigationPoint b = obj as DefaultNavigationPoint;
 			return this.FileName.CompareTo(b.FileName);
 		}
-		
-		#endregion
-		
-		#region Equality		
 
-		public override bool Equals(object obj)
+		// Omitting any of the following operator overloads
+		// violates rule: OverrideMethodsOnComparableTypes.
+		public static bool operator == (DefaultNavigationPoint p1, DefaultNavigationPoint p2)
 		{
-			DefaultNavigationPoint b = obj as DefaultNavigationPoint;
-			if (b == null) return false;
-			return this.FileName == b.FileName;
+			return p1==null ? p2==null : p1.Equals(p2);
 		}
-		
-		public override int GetHashCode()
+		public static bool operator != (DefaultNavigationPoint p1, DefaultNavigationPoint p2)
 		{
-			return this.FileName.GetHashCode();
+			return !(p1==p2);
+		}
+		public static bool operator < (DefaultNavigationPoint p1, DefaultNavigationPoint p2)
+		{
+			return p1==null ? p2!=null : (p1.CompareTo(p2) < 0);
+		}
+		public static bool operator > (DefaultNavigationPoint p1, DefaultNavigationPoint p2)
+		{
+			return p1==null ? false : (p1.CompareTo(p2) > 0);
 		}
 		#endregion
+		
 	}
 }
 

src/Main/Base/Project/Src/Services/NavigationService/NavigationService.cs

@@ -19,41 +19,41 @@ namespace ICSharpCode.Core
 	/// Provides the infrastructure to handle generalized code navigation.
 	/// </summary>
 	/// <remarks>
-	/// <para>This service is not limited to navigating code; rather, it 
-	/// integrates with SharpDevelop's extendable <see cref="IViewContent"/> 
-	/// interface so that each window has the opportunity to implement a 
+	/// <para>This service is not limited to navigating code; rather, it
+	/// integrates with SharpDevelop's extendable <see cref="IViewContent"/>
+	/// interface so that each window has the opportunity to implement a
 	/// contextually appropriate navigation scheme.</para>
-	/// <para>The default scheme, <see cref="DefaultNavigationPoint"/>, is 
-	/// created automatically in the <see cref="AbstractViewContent"/> 
-	/// implementation.  This scheme supports the basic function of logging a 
+	/// <para>The default scheme, <see cref="DefaultNavigationPoint"/>, is
+	/// created automatically in the <see cref="AbstractViewContent"/>
+	/// implementation.  This scheme supports the basic function of logging a
 	/// filename and returning to that file's default view.</para>
-	/// <para>The default text editor provides a slightly more sophisticated 
+	/// <para>The default text editor provides a slightly more sophisticated
 	/// scheme, <see cref="ICSharpCode.SharpDevelop.DefaultEditor.Gui.Editor.TextEditorNavigationPoint">
 	/// TextEditorNavigationPoint</see>, that logs filename and line number.</para>
-	/// <para>To implement your own navigation scheme, implement 
-	/// <see cref="IViewContent"/> or derive from 
-	/// <see cref="AbstractViewContent"/> and override the 
-	/// <see cref="IViewContent.BuildNavigationPoint">BuildNavigationPoint</see> 
+	/// <para>To implement your own navigation scheme, implement
+	/// <see cref="IViewContent"/> or derive from
+	/// <see cref="AbstractViewContent"/> and override the
+	/// <see cref="IViewContent.BuildNavigationPoint">BuildNavigationPoint</see>
 	/// method.</para>
 	/// <para>
 	/// <i>History logic based in part on Orlando Curioso's <i>Code Project</i> article:
 	/// <see href="http://www.codeproject.com/cs/miscctrl/WinFormsHistory.asp">
 	/// Navigational history (go back/forward) for WinForms controls</see></i>
-	/// </para>	
+	/// </para>
 	/// </remarks>
 	public class NavigationService
 	{
 		#region Private members
-		static LinkedList<INavigationPoint> history;
-		static LinkedListNode<INavigationPoint> currentNode;
-		static bool loggingSuspended;
+		static LinkedList<INavigationPoint> history = new LinkedList<INavigationPoint>();
+		static LinkedListNode<INavigationPoint> currentNode; // autoinitialized to null (FxCop)
+		static bool loggingSuspended; // autoinitialized to false (FxCop)
 		#endregion
 		
 		static NavigationService()
 		{
-			history = new LinkedList<INavigationPoint>();
-			currentNode = null;
-			loggingSuspended = false;
+//			history = new LinkedList<INavigationPoint>();
+//			currentNode = null;
+//			loggingSuspended = false;
 			
 			WorkbenchSingleton.WorkbenchCreated += WorkbenchCreatedHandler;
 			FileService.FileRenamed += FileService_FileRenamed;
@@ -86,7 +86,8 @@ namespace ICSharpCode.Core
 		#endregion
 
 		#region Public Methods
-	
+		
+		// TODO: FxCop says "find another way to do this" (ReviewVisibleEventHandlers)
 		public static void ContentChanging(object sender, EventArgs e)
 		{
 			foreach (INavigationPoint p in history)
@@ -110,12 +111,12 @@ namespace ICSharpCode.Core
 		}
 		#endregion
 		
-		public static void Log(INavigationPoint p)
+		public static void Log(INavigationPoint pointToLog)
 		{
 			if (loggingSuspended) {
 				return;
 			}
-			LogInternal(p);
+			LogInternal(pointToLog);
 		}
 
 		// refactoring this out of Log() allows the NavigationService
@@ -124,7 +125,7 @@ namespace ICSharpCode.Core
 		private static void LogInternal(INavigationPoint p)
 		{
 			if (p == null
-			   || p.FileName==null) { // HACK: why/how do we get here?
+			    || p.FileName==null) { // HACK: why/how do we get here?
 				return;
 			}
 			if (currentNode==null) {
@@ -155,9 +156,11 @@ namespace ICSharpCode.Core
 			return null;
 		}
 		
-		public static List<INavigationPoint> GetListOfPoints()
+		public static ICollection<INavigationPoint> Points
 		{
-			return new List<INavigationPoint>(history);
+			get {
+				return new List<INavigationPoint>(history);
+			}
 		}
 		
 		public static void ClearHistory()
@@ -176,21 +179,21 @@ namespace ICSharpCode.Core
 			OnHistoryChanged();
 		}
 		
-		public static void Go(int n)
+		public static void Go(int delta)
 		{
-			if (0 == n) {
+			if (0 == delta) {
 				return;
-			} else if (0>n) {
+			} else if (0>delta) {
 				// move backwards
-				while (0>n && currentNode!=history.First) {
+				while (0>delta && currentNode!=history.First) {
 					currentNode = currentNode.Previous;
-					n++;
+					delta++;
 				}
 			} else {
 				// move forwards
-				while (0<n && currentNode!=history.Last) {
+				while (0<delta && currentNode!=history.Last) {
 					currentNode = currentNode.Next;
-					n--;
+					delta--;
 				}
 			}
 			
@@ -220,7 +223,7 @@ namespace ICSharpCode.Core
 		{
 			//LoggingService.Info("suspend logging");
 			SuspendLogging();
-			if (CurrentPosition!=null) { 
+			if (CurrentPosition!=null) {
 				CurrentPosition.JumpTo();
 			}
 			//LoggingService.Info("resume logging");
@@ -241,14 +244,14 @@ namespace ICSharpCode.Core
 		
 		#endregion
 
-		// the following code is not covered by Unit tests as i wasn't sure 
+		// the following code is not covered by Unit tests as i wasn't sure
 		// how to test code triggered by the user interacting with the workbench
 		#region event trapping
 
 		#region ViewContent events
-		public static void WorkbenchCreatedHandler(object sender, EventArgs e)
+		static void WorkbenchCreatedHandler(object sender, EventArgs e)
 		{
-			WorkbenchSingleton.Workbench.ViewOpened += 
+			WorkbenchSingleton.Workbench.ViewOpened +=
 				new ViewContentEventHandler(ViewContentOpened);
 			WorkbenchSingleton.Workbench.ViewClosed +=
 				new ViewContentEventHandler(ViewContentClosed);
@@ -265,7 +268,7 @@ namespace ICSharpCode.Core
 			e.Content.WorkbenchWindow.WindowSelected -= WorkBenchWindowSelected;
 		}
 		
-		static IWorkbenchWindow lastSelectedWindow = null;
+		static IWorkbenchWindow lastSelectedWindow; // = null; (FXCop)
 		static void WorkBenchWindowSelected(object sender, EventArgs e)
 		{
 			try {
@@ -273,18 +276,19 @@ namespace ICSharpCode.Core
 				if (window == lastSelectedWindow) {
 					return;
 				}
-				int n = NavigationService.Count;
+				//int n = NavigationService.Count;
 				Log(window);
 				//LoggingService.DebugFormatted("WorkbenchSelected: logging {0}", window.Title);
 
-				// HACK: Navigation - for some reason, JumpToFilePosition returns _before_ this 
-				//       gets fired, (not the behaviour i expected) so we need to remember the 
-				//       previous selected window to ensure that we only log once per visit to 
+				// HACK: Navigation - for some reason, JumpToFilePosition returns _before_ this
+				//       gets fired, (not the behaviour i expected) so we need to remember the
+				//       previous selected window to ensure that we only log once per visit to
 				//       a given window.
 				lastSelectedWindow = window;
 
 			} catch (Exception ex) {
 				LoggingService.ErrorFormatted("{0}:\n{1}",ex.Message, ex.StackTrace);
+				throw;
 			}
 		}
 		#endregion
@@ -304,7 +308,7 @@ namespace ICSharpCode.Core
 		#region Public Events
 		public static event System.EventHandler HistoryChanged;
 		
-		public static void OnHistoryChanged()
+		static void OnHistoryChanged()
 		{
 			if (HistoryChanged!=null) {
 				HistoryChanged(NavigationService.CurrentPosition, EventArgs.Empty);

src/Main/Base/Test/Services_Navigation/NavigationServiceTestFixture.cs

@@ -104,15 +104,15 @@ namespace NavigationServiceTests
 		[Test]
 		/// <summary>
 		/// The <see cref="NavigationService"/> must
-		/// expose a list of points in it's history.
-		/// <see cref="List<T>"/> of type
+		/// expose a list of points in it's history:
+		/// <see cref="ICollection<T>"/> of type
 		/// <see cref="INavigationPoint"/>.
 		/// </summary>
 		/// <remarks>necessary for testing and for menu building</remarks>
 		public void GetListOfPointsTest()
 		{
-			Assert.IsInstanceOfType(typeof(List<INavigationPoint>),
-			                        NavigationService.GetListOfPoints());
+			Assert.IsInstanceOfType(typeof(ICollection<INavigationPoint>),
+			                        NavigationService.Points;
 		}
 		
 		[Test]
@@ -536,6 +536,7 @@ namespace NavigationServiceTests
 		}
 		
 		[Test]
+		[Ignore] // this test disabled on purpose - DA
 		/// <summary>
 		/// The <see cref="NavigationService"/> must
 		/// ignore requests to log a point equivalent

src/SharpDevelop.Tests.sln

@@ -1,9 +1,11 @@
 Microsoft Visual Studio Solution File, Format Version 9.00
-# SharpDevelop 2.1.0.1472
+# SharpDevelop 2.1.0.1574
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "AddIns", "AddIns", "{14A277EE-7DF1-4529-B639-7D1EF334C1C5}"
 	ProjectSection(SolutionItems) = postProject
 	EndProjectSection
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CodeAnalysis", "AddIns\Misc\CodeAnalysis\CodeAnalysis.csproj", "{3EAA45A9-735C-4AC7-A799-947B93EA449D}"
+EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "FormsDesigner", "AddIns\DisplayBindings\FormsDesigner\Project\FormsDesigner.csproj", "{7D7E92DF-ACEB-4B69-92C8-8AC7A703CD57}"
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "BooBinding.Tests", "AddIns\BackendBindings\Boo\BooBinding\Test\BooBinding.Tests.csproj", "{6FA16499-896F-4C02-BB43-1AF5C6C7C713}"
@@ -228,6 +230,10 @@ Global
 		{B3EFF50A-ADBF-4FCD-ACE3-081AF1B37F1D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{B3EFF50A-ADBF-4FCD-ACE3-081AF1B37F1D}.Release|Any CPU.Build.0 = Release|Any CPU
 		{B3EFF50A-ADBF-4FCD-ACE3-081AF1B37F1D}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{3EAA45A9-735C-4AC7-A799-947B93EA449D}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{3EAA45A9-735C-4AC7-A799-947B93EA449D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{3EAA45A9-735C-4AC7-A799-947B93EA449D}.Release|Any CPU.Build.0 = Release|Any CPU
+		{3EAA45A9-735C-4AC7-A799-947B93EA449D}.Release|Any CPU.ActiveCfg = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -250,6 +256,7 @@ Global
 		{4AC2D5F1-F671-480C-A075-6BF62B3721B2} = {14A277EE-7DF1-4529-B639-7D1EF334C1C5}
 		{6FA16499-896F-4C02-BB43-1AF5C6C7C713} = {14A277EE-7DF1-4529-B639-7D1EF334C1C5}
 		{7D7E92DF-ACEB-4B69-92C8-8AC7A703CD57} = {14A277EE-7DF1-4529-B639-7D1EF334C1C5}
+		{3EAA45A9-735C-4AC7-A799-947B93EA449D} = {14A277EE-7DF1-4529-B639-7D1EF334C1C5}
 		{B08385CD-F0CC-488C-B4F4-EEB34B6D2688} = {6604365C-C702-4C10-9BA8-637F1E3D4D0D}
 		{1D18D788-F7EE-4585-A23B-34DC8EC63CB8} = {6604365C-C702-4C10-9BA8-637F1E3D4D0D}
 		{EC06F96A-AEEC-49D6-B03D-AB87C6EB674C} = {6604365C-C702-4C10-9BA8-637F1E3D4D0D}