Merge pull request #30 from github/haacked/code-cleanup

shana · shana · commit 391151c0f8dd · 2015-08-05T09:08:15.000-04:00
A bit of tidying up
diff --git a/src/CredentialManagement/CredentialSet.cs b/src/CredentialManagement/CredentialSet.cs
@@ -18,7 +18,7 @@ public CredentialSet()
         public CredentialSet(string target)
             : this()
         {
-            Guard.ArgumentNotEmptyString(target, "target");
+            Guard.ArgumentNotEmptyString(target, nameof(target));
 
             Target = target;
         }
diff --git a/src/GitHub.App/Api/ApiClient.cs b/src/GitHub.App/Api/ApiClient.cs
@@ -46,7 +46,7 @@ public ApiClient(HostAddress hostAddress, IObservableGitHubClient gitHubClient)
 
         public IObservable<Repository> CreateRepository(NewRepository repository, string login, bool isUser)
         {
-            Guard.ArgumentNotEmptyString(login, "login");
+            Guard.ArgumentNotEmptyString(login, nameof(login));
 
             var client = gitHubClient.Repository;
 
diff --git a/src/GitHub.App/Caches/ImageCache.cs b/src/GitHub.App/Caches/ImageCache.cs
@@ -239,7 +239,6 @@ void Dispose(bool disposing)
         public void Dispose()
         {
             Dispose(true);
-            GC.SuppressFinalize(this);
         }
 
         class UriComparer : IEqualityComparer<Uri>
diff --git a/src/GitHub.App/Caches/LoginCache.cs b/src/GitHub.App/Caches/LoginCache.cs
@@ -37,8 +37,8 @@ public IObservable<LoginInfo> GetLoginAsync(HostAddress hostAddress)
 
         public IObservable<Unit> SaveLogin(string user, string password, HostAddress hostAddress)
         {
-            Guard.ArgumentNotEmptyString(user, "user");
-            Guard.ArgumentNotEmptyString(password, "password");
+            Guard.ArgumentNotEmptyString(user, nameof(user));
+            Guard.ArgumentNotEmptyString(password, nameof(password));
 
             return cache.Secure.SaveLogin(user, password, hostAddress.CredentialCacheKeyHost);
         }
@@ -78,7 +78,6 @@ void Dispose(bool disposing)
         public void Dispose()
         {
             Dispose(true);
-            GC.SuppressFinalize(this);
         }
     }
 }
diff --git a/src/GitHub.App/Factories/SqlitePersistentBlobCacheFactory.cs b/src/GitHub.App/Factories/SqlitePersistentBlobCacheFactory.cs
@@ -14,7 +14,7 @@ public class SqlitePersistentBlobCacheFactory : IBlobCacheFactory
 
         public IBlobCache CreateBlobCache(string path)
         {
-            Guard.ArgumentNotEmptyString(path, "path");
+            Guard.ArgumentNotEmptyString(path, nameof(path));
 
             try
             {
diff --git a/src/GitHub.App/Models/RepositoryHost.cs b/src/GitHub.App/Models/RepositoryHost.cs
@@ -28,7 +28,7 @@ public class RepositoryHost : ReactiveObject, IRepositoryHost
         readonly ILoginCache loginCache;
 
         bool isLoggedIn;
-        bool isEnterprise;
+        readonly bool isEnterprise;
 
         public RepositoryHost(
             IApiClient apiClient,
@@ -86,8 +86,8 @@ public IObservable<AuthenticationResult> LogInFromCache()
 
         public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string password)
         {
-            Guard.ArgumentNotEmptyString(usernameOrEmail, "usernameOrEmail");
-            Guard.ArgumentNotEmptyString(password, "password");
+            Guard.ArgumentNotEmptyString(usernameOrEmail, nameof(usernameOrEmail));
+            Guard.ArgumentNotEmptyString(password, nameof(password));
 
             // If we need to retry on fallback, we'll store the 2FA token 
             // from the first request to re-use:
diff --git a/src/GitHub.App/SampleData/SampleViewModels.cs b/src/GitHub.App/SampleData/SampleViewModels.cs
@@ -300,7 +300,6 @@ public string Title
 
         public void Dispose()
         {
-            GC.SuppressFinalize(this);
         }
 
         public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string password)
diff --git a/src/GitHub.App/Services/GitClient.cs b/src/GitHub.App/Services/GitClient.cs
@@ -13,8 +13,8 @@ public class GitClient : IGitClient
     {
         public IObservable<Unit> Push(IRepository repository, string branchName, string remoteName)
         {
-            Guard.ArgumentNotEmptyString(branchName, "branchName");
-            Guard.ArgumentNotEmptyString(remoteName, "remoteName");
+            Guard.ArgumentNotEmptyString(branchName, nameof(branchName));
+            Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));
 
             return Observable.Defer(() =>
             {
@@ -32,7 +32,7 @@ public IObservable<Unit> Push(IRepository repository, string branchName, string
 
         public IObservable<Unit> Fetch(IRepository repository, string remoteName)
         {
-            Guard.ArgumentNotEmptyString(remoteName, "remoteName");
+            Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));
 
             return Observable.Defer(() =>
             {
@@ -44,7 +44,7 @@ public IObservable<Unit> Fetch(IRepository repository, string remoteName)
 
         public IObservable<Unit> SetRemote(IRepository repository, string remoteName, Uri url)
         {
-            Guard.ArgumentNotEmptyString(remoteName, "remoteName");
+            Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));
 
             return Observable.Defer(() =>
             {
@@ -57,8 +57,8 @@ public IObservable<Unit> SetRemote(IRepository repository, string remoteName, Ur
 
         public IObservable<Unit> SetTrackingBranch(IRepository repository, string branchName, string remoteName)
         {
-            Guard.ArgumentNotEmptyString(branchName, "branchName");
-            Guard.ArgumentNotEmptyString(remoteName, "remoteName");
+            Guard.ArgumentNotEmptyString(branchName, nameof(branchName));
+            Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));
 
             return Observable.Defer(() =>
             {
diff --git a/src/GitHub.App/Services/ModelService.cs b/src/GitHub.App/Services/ModelService.cs
@@ -110,7 +110,7 @@ IObservable<IEnumerable<AccountCacheItem>> GetUserOrganizations()
                     // This could in theory happen if we try to call this before the user is logged in.
                     e =>
                     {
-                        log.Error("Retrieve user organizations failed because user is not stored in the cache.", e);
+                        log.Error("Retrieve user organizations failed because user is not stored in the cache.", (Exception)e);
                         return Observable.Return(Enumerable.Empty<AccountCacheItem>());
                     })
                  .Catch<IEnumerable<AccountCacheItem>, Exception>(e =>
diff --git a/src/GitHub.App/Services/RepositoryCloneService.cs b/src/GitHub.App/Services/RepositoryCloneService.cs
@@ -35,9 +35,9 @@ public RepositoryCloneService(IOperatingSystem operatingSystem, IVSServices vsse
 
         public IObservable<Unit> CloneRepository(string cloneUrl, string repositoryName, string repositoryPath)
         {
-            Guard.ArgumentNotEmptyString(cloneUrl, "cloneUrl");
-            Guard.ArgumentNotEmptyString(repositoryName, "repositoryName");
-            Guard.ArgumentNotEmptyString(repositoryPath, "repositoryPath");
+            Guard.ArgumentNotEmptyString(cloneUrl, nameof(cloneUrl));
+            Guard.ArgumentNotEmptyString(repositoryName, nameof(repositoryName));
+            Guard.ArgumentNotEmptyString(repositoryPath, nameof(repositoryPath));
 
             return Observable.Start(() =>
             {
@@ -53,7 +53,7 @@ public IObservable<Unit> CloneRepository(string cloneUrl, string repositoryName,
                 catch (Exception ex)
                 {
                     log.Error("Could not clone {0} to {1}. {2}", cloneUrl, path, ex);
-                    throw ex;
+                    throw;
                 }
                 
                 return Unit.Default;
diff --git a/src/GitHub.App/Services/RepositoryCreationService.cs b/src/GitHub.App/Services/RepositoryCreationService.cs
@@ -32,7 +32,7 @@ public IObservable<Unit> CreateRepository(
             string directory,
             IApiClient apiClient)
         {
-            Guard.ArgumentNotEmptyString(directory, "directory");
+            Guard.ArgumentNotEmptyString(directory, nameof(directory));
 
             return apiClient.CreateRepository(newRepository, account.Login, account.IsUser)
                 .Select(repository => cloneService.CloneRepository(repository.CloneUrl, repository.Name, directory))
diff --git a/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs b/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs
@@ -15,7 +15,7 @@ namespace GitHub.ViewModels
     [PartCreationPolicy(CreationPolicy.NonShared)]
     public class LoginToGitHubViewModel : LoginTabViewModel, ILoginToGitHubViewModel
     {
-        Uri baseUri;
+        readonly Uri baseUri;
 
         [ImportingConstructor]
         public LoginToGitHubViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrowser browser)
diff --git a/src/GitHub.Exports.Reactive/Caches/IImageCache.cs b/src/GitHub.Exports.Reactive/Caches/IImageCache.cs
@@ -10,7 +10,7 @@ public interface IImageCache : IDisposable
         /// Retrieves the image from the cache. If it's missing, it'll download it and cache it.
         /// </summary>
         /// <param name="url">The image Uri to fetch.</param>
-        /// <exception cref="KeyNotFoundException">Thrown if an image is missing and could not be fetched.</exception>
+        /// <exception cref="System.Collections.Generic.KeyNotFoundException">Thrown if an image is missing and could not be fetched.</exception>
         IObservable<BitmapSource> GetImage(Uri url);
 
         /// <summary>
diff --git a/src/GitHub.Exports.Reactive/Helpers/Guard.cs b/src/GitHub.Exports.Reactive/Helpers/Guard.cs
@@ -10,7 +10,8 @@ public static class Guard
         /// <summary>
         /// Validates that the string is not empty.
         /// </summary>
-        /// <param name="value"></param>
+        /// <param name="value">The string to check.</param>
+        /// <param name="name">The name of the argument</param>
         public static void ArgumentNotEmptyString(string value, string name)
         {
             // We already know the value is not null because of NullGuard.Fody.
diff --git a/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs b/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs
@@ -19,7 +19,7 @@ public interface IRepositoryCloneService
         /// </summary>
         /// <param name="cloneUrl">The url of the repository to clone.</param>
         /// <param name="repositoryName">The name of the repository to clone.</param>
-        /// <param name="repositoryParentDirectory">The directory that will contain the repository directory.</param>
+        /// <param name="repositoryPath">The directory that will contain the repository directory.</param>
         /// <returns></returns>
         IObservable<Unit> CloneRepository(string cloneUrl, string repositoryName, string repositoryPath);
     }
diff --git a/src/GitHub.Extensions.Reactive/ObservableExtensions.cs b/src/GitHub.Extensions.Reactive/ObservableExtensions.cs
@@ -180,9 +180,6 @@ public static IObservable<IReadOnlyList<T>> ToReadOnlyList<T>(this IObservable<T
         /// <summary>
         /// Aggregates the items in the observable sequence into a readonly list.
         /// </summary>
-        /// <typeparam name="T"></typeparam>
-        /// <param name="source"></param>
-        /// <returns></returns>
         public static IObservable<IReadOnlyList<TResult>> ToReadOnlyList<T, TResult>(this IObservable<IEnumerable<T>> source, Func<T, TResult> map)
         {
             return source.Select(items => new ReadOnlyCollection<TResult>(items.Select(map).ToList()));
@@ -191,9 +188,6 @@ public static IObservable<IReadOnlyList<TResult>> ToReadOnlyList<T, TResult>(thi
         /// <summary>
         /// Aggregates the items in the observable sequence into a readonly list.
         /// </summary>
-        /// <typeparam name="T"></typeparam>
-        /// <param name="source"></param>
-        /// <returns></returns>
         public static IObservable<IReadOnlyList<TResult>> ToReadOnlyList<T, TResult>(this IObservable<IEnumerable<T>> source, Func<T, TResult> map, TResult firstItem)
         {
             return source.Select(items => new ReadOnlyCollection<TResult>(new[] { firstItem }.Concat(items.Select(map)).ToList()));
diff --git a/src/GitHub.UI.Reactive/Controls/Validation/ValidationMessage.cs b/src/GitHub.UI.Reactive/Controls/Validation/ValidationMessage.cs
@@ -125,9 +125,9 @@ void ShowValidateError(bool showError)
 
             var bindingExpression = ValidatesControl.GetBindingExpression(TextBox.TagProperty);
             if (bindingExpression == null) return;
-            var validationError = new ValidationError(
-                new ExceptionValidationRule(),
-                BindingOperations.GetBindingExpression(ValidatesControl, TextBox.TagProperty));
+            var opExpression = BindingOperations.GetBindingExpression(ValidatesControl, TextBox.TagProperty);
+            if (opExpression == null) return;
+            var validationError = new ValidationError(new ExceptionValidationRule(), opExpression);
 
             if (showError)
             {
diff --git a/src/GitHub.UI.Reactive/Validation/ReactiveValidatableObject.cs b/src/GitHub.UI.Reactive/Validation/ReactiveValidatableObject.cs
@@ -132,8 +132,8 @@ string GetErrorMessage(string propertyName)
 
         public void SetErrorMessage(string propertyName, string errorMessage)
         {
-            Guard.ArgumentNotEmptyString(propertyName, "propertyName");
-            Guard.ArgumentNotEmptyString(errorMessage, "errorMessage");
+            Guard.ArgumentNotEmptyString(propertyName, nameof(propertyName));
+            Guard.ArgumentNotEmptyString(errorMessage, nameof(errorMessage));
 
             ValidatedProperty validatedProperty;
             if (!validatedProperties.TryGetValue(propertyName, out validatedProperty))
diff --git a/src/GitHub.UI.Reactive/Validation/ValidateIfAttribute.cs b/src/GitHub.UI.Reactive/Validation/ValidateIfAttribute.cs
@@ -18,7 +18,7 @@ public sealed class ValidateIfAttribute : ValidationAttribute
     {
         public ValidateIfAttribute(string dependentPropertyName)
         {
-            Guard.ArgumentNotEmptyString(dependentPropertyName, "dependentPropertyName");
+            Guard.ArgumentNotEmptyString(dependentPropertyName, nameof(dependentPropertyName));
 
             DependentPropertyName = dependentPropertyName;
         }
diff --git a/src/GitHub.UI/Controls/FilterTextBox.cs b/src/GitHub.UI/Controls/FilterTextBox.cs
@@ -22,8 +22,6 @@ public string PromptText
             set { SetValue(PromptTextProperty, value); }
         }
 
-        public ICommand ClearCommand { get; private set; }
-
         public FilterTextBox()
         {
             // http://stackoverflow.com/a/661224/2114
@@ -66,11 +64,9 @@ static void SelectivelyIgnoreMouseButton(object sender, MouseButtonEventArgs e)
 
         static TextBox FindTextBoxInAncestors(DependencyObject current)
         {
-            TextBox tb;
-
             while (current != null)
             {
-                tb = current as TextBox;
+                var tb = current as TextBox;
                 if (tb != null)
                     return tb;
 

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ public CredentialSet()`
`18`	`18`	`public CredentialSet(string target)`
`19`	`19`	`: this()`
`20`	`20`	`{`
`21`		`- Guard.ArgumentNotEmptyString(target, "target");`
	`21`	`+ Guard.ArgumentNotEmptyString(target, nameof(target));`
`22`	`22`
`23`	`23`	`Target = target;`
`24`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ public ApiClient(HostAddress hostAddress, IObservableGitHubClient gitHubClient)`
`46`	`46`
`47`	`47`	`public IObservable<Repository> CreateRepository(NewRepository repository, string login, bool isUser)`
`48`	`48`	`{`
`49`		`- Guard.ArgumentNotEmptyString(login, "login");`
	`49`	`+ Guard.ArgumentNotEmptyString(login, nameof(login));`
`50`	`50`
`51`	`51`	`var client = gitHubClient.Repository;`
`52`	`52`
Original file line number	Diff line number	Diff line change
`@@ -239,7 +239,6 @@ void Dispose(bool disposing)`
`239`	`239`	`public void Dispose()`
`240`	`240`	`{`
`241`	`241`	`Dispose(true);`
`242`		`- GC.SuppressFinalize(this);`
`243`	`242`	`}`
`244`	`243`
`245`	`244`	`class UriComparer : IEqualityComparer<Uri>`
Original file line number	Diff line number	Diff line change
`@@ -37,8 +37,8 @@ public IObservable<LoginInfo> GetLoginAsync(HostAddress hostAddress)`
`37`	`37`
`38`	`38`	`public IObservable<Unit> SaveLogin(string user, string password, HostAddress hostAddress)`
`39`	`39`	`{`
`40`		`- Guard.ArgumentNotEmptyString(user, "user");`
`41`		`- Guard.ArgumentNotEmptyString(password, "password");`
	`40`	`+ Guard.ArgumentNotEmptyString(user, nameof(user));`
	`41`	`+ Guard.ArgumentNotEmptyString(password, nameof(password));`
`42`	`42`
`43`	`43`	`return cache.Secure.SaveLogin(user, password, hostAddress.CredentialCacheKeyHost);`
`44`	`44`	`}`
`@@ -78,7 +78,6 @@ void Dispose(bool disposing)`
`78`	`78`	`public void Dispose()`
`79`	`79`	`{`
`80`	`80`	`Dispose(true);`
`81`		`- GC.SuppressFinalize(this);`
`82`	`81`	`}`
`83`	`82`	`}`
`84`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ public class SqlitePersistentBlobCacheFactory : IBlobCacheFactory`
`14`	`14`
`15`	`15`	`public IBlobCache CreateBlobCache(string path)`
`16`	`16`	`{`
`17`		`- Guard.ArgumentNotEmptyString(path, "path");`
	`17`	`+ Guard.ArgumentNotEmptyString(path, nameof(path));`
`18`	`18`
`19`	`19`	`try`
`20`	`20`	`{`
Original file line number	Diff line number	Diff line change
`@@ -300,7 +300,6 @@ public string Title`
`300`	`300`
`301`	`301`	`public void Dispose()`
`302`	`302`	`{`
`303`		`- GC.SuppressFinalize(this);`
`304`	`303`	`}`
`305`	`304`
`306`	`305`	`public IObservable<AuthenticationResult> LogIn(string usernameOrEmail, string password)`
Original file line number	Diff line number	Diff line change
`@@ -13,8 +13,8 @@ public class GitClient : IGitClient`
`13`	`13`	`{`
`14`	`14`	`public IObservable<Unit> Push(IRepository repository, string branchName, string remoteName)`
`15`	`15`	`{`
`16`		`- Guard.ArgumentNotEmptyString(branchName, "branchName");`
`17`		`- Guard.ArgumentNotEmptyString(remoteName, "remoteName");`
	`16`	`+ Guard.ArgumentNotEmptyString(branchName, nameof(branchName));`
	`17`	`+ Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));`
`18`	`18`
`19`	`19`	`return Observable.Defer(() =>`
`20`	`20`	`{`
`@@ -32,7 +32,7 @@ public IObservable<Unit> Push(IRepository repository, string branchName, string`
`32`	`32`
`33`	`33`	`public IObservable<Unit> Fetch(IRepository repository, string remoteName)`
`34`	`34`	`{`
`35`		`- Guard.ArgumentNotEmptyString(remoteName, "remoteName");`
	`35`	`+ Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));`
`36`	`36`
`37`	`37`	`return Observable.Defer(() =>`
`38`	`38`	`{`
`@@ -44,7 +44,7 @@ public IObservable<Unit> Fetch(IRepository repository, string remoteName)`
`44`	`44`
`45`	`45`	`public IObservable<Unit> SetRemote(IRepository repository, string remoteName, Uri url)`
`46`	`46`	`{`
`47`		`- Guard.ArgumentNotEmptyString(remoteName, "remoteName");`
	`47`	`+ Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));`
`48`	`48`
`49`	`49`	`return Observable.Defer(() =>`
`50`	`50`	`{`
`@@ -57,8 +57,8 @@ public IObservable<Unit> SetRemote(IRepository repository, string remoteName, Ur`
`57`	`57`
`58`	`58`	`public IObservable<Unit> SetTrackingBranch(IRepository repository, string branchName, string remoteName)`
`59`	`59`	`{`
`60`		`- Guard.ArgumentNotEmptyString(branchName, "branchName");`
`61`		`- Guard.ArgumentNotEmptyString(remoteName, "remoteName");`
	`60`	`+ Guard.ArgumentNotEmptyString(branchName, nameof(branchName));`
	`61`	`+ Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));`
`62`	`62`
`63`	`63`	`return Observable.Defer(() =>`
`64`	`64`	`{`
Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ IObservable<IEnumerable<AccountCacheItem>> GetUserOrganizations()`
`110`	`110`	`// This could in theory happen if we try to call this before the user is logged in.`
`111`	`111`	`e =>`
`112`	`112`	`{`
`113`		`- log.Error("Retrieve user organizations failed because user is not stored in the cache.", e);`
	`113`	`+ log.Error("Retrieve user organizations failed because user is not stored in the cache.", (Exception)e);`
`114`	`114`	`return Observable.Return(Enumerable.Empty<AccountCacheItem>());`
`115`	`115`	`})`
`116`	`116`	`.Catch<IEnumerable<AccountCacheItem>, Exception>(e =>`
Original file line number	Diff line number	Diff line change
`@@ -35,9 +35,9 @@ public RepositoryCloneService(IOperatingSystem operatingSystem, IVSServices vsse`
`35`	`35`
`36`	`36`	`public IObservable<Unit> CloneRepository(string cloneUrl, string repositoryName, string repositoryPath)`
`37`	`37`	`{`
`38`		`- Guard.ArgumentNotEmptyString(cloneUrl, "cloneUrl");`
`39`		`- Guard.ArgumentNotEmptyString(repositoryName, "repositoryName");`
`40`		`- Guard.ArgumentNotEmptyString(repositoryPath, "repositoryPath");`
	`38`	`+ Guard.ArgumentNotEmptyString(cloneUrl, nameof(cloneUrl));`
	`39`	`+ Guard.ArgumentNotEmptyString(repositoryName, nameof(repositoryName));`
	`40`	`+ Guard.ArgumentNotEmptyString(repositoryPath, nameof(repositoryPath));`
`41`	`41`
`42`	`42`	`return Observable.Start(() =>`
`43`	`43`	`{`
`@@ -53,7 +53,7 @@ public IObservable<Unit> CloneRepository(string cloneUrl, string repositoryName,`
`53`	`53`	`catch (Exception ex)`
`54`	`54`	`{`
`55`	`55`	`log.Error("Could not clone {0} to {1}. {2}", cloneUrl, path, ex);`
`56`		`- throw ex;`
	`56`	`+ throw;`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`return Unit.Default;`