Skip to content

Ensure lack of optional parameters #1031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
May 15, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
494de48
Drop optional parameters in TagCollection
shiftkey May 10, 2015
14b3c1a
Drop optional parameters in RepositoryExtensions
shiftkey May 10, 2015
a897011
Drop optional parameters in Repository.cs
shiftkey May 10, 2015
cb9ebe5
Drop optional parameters in RemoteCollection.cs
shiftkey May 10, 2015
0a030f8
Drop optional parameters in ReferenceCollectionExtensions.cs
shiftkey May 10, 2015
1bb9d42
Drop optional parameters in ObjectDatabase.cs
shiftkey May 10, 2015
d16fbec
Drop optional parameters in TagCollectionExtensions.cs
shiftkey May 10, 2015
bce3f4c
Drop optional parameters in ReferenceCollectionExtensions.cs
shiftkey May 10, 2015
631b36c
Drop optional parameters in NetworkExtensions.cs
shiftkey May 10, 2015
4277559
Drop optional parameters in ConfigurationExtensions.cs
shiftkey May 10, 2015
9b26f36
Drop optional parameters in Configuration.cs
shiftkey May 10, 2015
1f9d9dd
Drop optional parameters in StashCollection.cs
shiftkey May 10, 2015
0cfa923
Drop optional parameters in Network.cs
shiftkey May 10, 2015
d7e8a2e
Drop optional parameters in CommitRewriteInfo.cs
shiftkey May 10, 2015
b3386c9
Drop optional parameters in BranchCollectionExtensions.cs
shiftkey May 10, 2015
40bbf1e
Drop optional parameters in BranchCollection.cs
shiftkey May 10, 2015
32da01c
Drop optional parameters in BlobExtensions.cs
shiftkey May 10, 2015
44aa149
Drop optional parameters in Diff.cs
shiftkey May 10, 2015
5109f59
Ensure lack of optional parameters
shiftkey May 10, 2015
62cc73e
Fix Resharper private member prefix setting
shiftkey May 10, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion LibGit2Sharp.Tests/ConfigurationFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public void CanSetBooleanValue()
[Fact]
public void SettingLocalConfigurationOutsideAReposThrows()
{
using (var config = new Configuration())
using (var config = new Configuration(null, null, null))
{
Assert.Throws<LibGit2SharpException>(() => config.Set("unittests.intsetting", 3));
}
Expand Down
54 changes: 54 additions & 0 deletions LibGit2Sharp.Tests/MetaFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -299,5 +299,59 @@ public void NoPublicTypesUnderLibGit2SharpCoreNamespace()
Assert.True(false, Environment.NewLine + sb.ToString());
}
}

[Fact]
public void NoOptionalParametersinMethods()
{
IEnumerable<string> mis =
from t in Assembly.GetAssembly(typeof(IRepository))
.GetExportedTypes()
from m in t.GetMethods()
where !m.IsObsolete()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a couple of methods which have optional parameters but are marked as obsolete. I figure we don't care about migrating those. Let me know if that's not the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! They'll be dropped before stabilization (post v0.22)

from p in m.GetParameters()
where p.IsOptional
select m.DeclaringType + "." + m.Name;

var sb = new StringBuilder();

foreach (var method in mis.Distinct())
{
sb.AppendFormat("At least one overload of method '{0}' accepts an optional parameter.{1}",
method, Environment.NewLine);
}

Assert.Equal("", sb.ToString());
}

[Fact]
public void NoOptionalParametersinConstructors()
{
IEnumerable<string> mis =
from t in Assembly.GetAssembly(typeof(IRepository))
.GetExportedTypes()
from c in t.GetConstructors()
from p in c.GetParameters()
where p.IsOptional
select c.DeclaringType.Name;

var sb = new StringBuilder();

foreach (var method in mis.Distinct())
{
sb.AppendFormat("At least one constructor of type '{0}' accepts an optional parameter.{1}",
method, Environment.NewLine);
}

Assert.Equal("", sb.ToString());
}
}

internal static class TypeExtensions
{
internal static bool IsObsolete(this MethodInfo methodInfo)
{
var attributes = methodInfo.GetCustomAttributes(false);
return attributes.Any(a => a is ObsoleteAttribute);
}
}
}
2 changes: 2 additions & 0 deletions LibGit2Sharp.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
<s:Boolean x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/SPACE_AFTER_TYPECAST_PARENTHESES/@EntryValue">False</s:Boolean>
<s:Boolean x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/SPACE_WITHING_EMPTY_BRACES/@EntryValue">True</s:Boolean>
<s:Boolean x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/SPACE_WITHIN_SINGLE_LINE_ARRAY_INITIALIZER_BRACES/@EntryValue">True</s:Boolean>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this change do? (asking for a friend)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found myself dropping the default _ when R# was bugging me during the refactoring. I'll check.

<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAddAccessorOwnerDeclarationBracesMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
</wpf:ResourceDictionary>
33 changes: 28 additions & 5 deletions LibGit2Sharp/BlobExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,44 @@ namespace LibGit2Sharp
/// </summary>
public static class BlobExtensions
{
/// <summary>
/// Gets the blob content, decoded with UTF8 encoding if the encoding cannot be detected from the byte order mark
/// </summary>
/// <param name="blob">The blob for which the content will be returned.</param>
/// <returns>Blob content as text.</returns>
public static string GetContentText(this Blob blob)
{
Ensure.ArgumentNotNull(blob, "blob");

return ReadToEnd(blob.GetContentStream(), null);
}

/// <summary>
/// Gets the blob content decoded with the specified encoding,
/// or according to byte order marks, with UTF8 as fallback,
/// if <paramref name="encoding"/> is null.
/// or according to byte order marks, or the specified encoding as a fallback
/// </summary>
/// <param name="blob">The blob for which the content will be returned.</param>
/// <param name="encoding">The encoding of the text. (default: detected or UTF8)</param>
/// <param name="encoding">The encoding of the text to use, if it cannot be detected</param>
/// <returns>Blob content as text.</returns>
public static string GetContentText(this Blob blob, Encoding encoding = null)
public static string GetContentText(this Blob blob, Encoding encoding)
{
Ensure.ArgumentNotNull(blob, "blob");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure.ArgumentNotNull(encoding, "encoding");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Ensure.ArgumentNotNull(encoding, "encoding");

return ReadToEnd(blob.GetContentStream(), encoding);
}

/// <summary>
/// Gets the blob content, decoded with UTF8 encoding if the encoding cannot be detected
/// </summary>
/// <param name="blob">The blob for which the content will be returned.</param>
/// <param name="filteringOptions">Parameter controlling content filtering behavior</param>
/// <returns>Blob content as text.</returns>
public static string GetContentText(this Blob blob, FilteringOptions filteringOptions)
{
return blob.GetContentText(filteringOptions, null);
}

/// <summary>
/// Gets the blob content as it would be checked out to the
/// working directory, decoded with the specified encoding,
Expand All @@ -34,7 +57,7 @@ public static string GetContentText(this Blob blob, Encoding encoding = null)
/// <param name="filteringOptions">Parameter controlling content filtering behavior</param>
/// <param name="encoding">The encoding of the text. (default: detected or UTF8)</param>
/// <returns>Blob content as text.</returns>
public static string GetContentText(this Blob blob, FilteringOptions filteringOptions, Encoding encoding = null)
public static string GetContentText(this Blob blob, FilteringOptions filteringOptions, Encoding encoding)
{
Ensure.ArgumentNotNull(blob, "blob");
Ensure.ArgumentNotNull(filteringOptions, "filteringOptions");
Expand Down
26 changes: 24 additions & 2 deletions LibGit2Sharp/BranchCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,25 @@ IEnumerator IEnumerable.GetEnumerator()

#endregion

/// <summary>
/// Create a new local branch with the specified name
/// </summary>
/// <param name="name">The name of the branch.</param>
/// <param name="committish">Revparse spec for the target commit.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public virtual Branch Add(string name, string committish)
{
return Add(name, committish, false);
}

/// <summary>
/// Create a new local branch with the specified name
/// </summary>
/// <param name="name">The name of the branch.</param>
/// <param name="committish">Revparse spec for the target commit.</param>
/// <param name="allowOverwrite">True to allow silent overwriting a potentially existing branch, false otherwise.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public virtual Branch Add(string name, string committish, bool allowOverwrite = false)
public virtual Branch Add(string name, string committish, bool allowOverwrite)
{
Ensure.ArgumentNotNullOrEmptyString(name, "name");
Ensure.ArgumentNotNullOrEmptyString(committish, "committish");
Expand All @@ -138,14 +149,25 @@ public virtual void Remove(Branch branch)
}
}

/// <summary>
/// Rename an existing local branch
/// </summary>
/// <param name="branch">The current local branch.</param>
/// <param name="newName">The new name the existing branch should bear.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public virtual Branch Rename(Branch branch, string newName)
{
return Rename(branch, newName, false);
}

/// <summary>
/// Rename an existing local branch
/// </summary>
/// <param name="branch">The current local branch.</param>
/// <param name="newName">The new name the existing branch should bear.</param>
/// <param name="allowOverwrite">True to allow silent overwriting a potentially existing branch, false otherwise.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public virtual Branch Rename(Branch branch, string newName, bool allowOverwrite = false)
public virtual Branch Rename(Branch branch, string newName, bool allowOverwrite)
{
Ensure.ArgumentNotNull(branch, "branch");
Ensure.ArgumentNotNullOrEmptyString(newName, "newName");
Expand Down
39 changes: 36 additions & 3 deletions LibGit2Sharp/BranchCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@ namespace LibGit2Sharp
/// </summary>
public static class BranchCollectionExtensions
{
/// <summary>
/// Create a new local branch with the specified name
/// </summary>
/// <param name="branches">The <see cref="BranchCollection"/> being worked with.</param>
/// <param name="name">The name of the branch.</param>
/// <param name="commit">The target commit.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public static Branch Add(this BranchCollection branches, string name, Commit commit)
{
return branches.Add(name, commit, false);
}

/// <summary>
/// Create a new local branch with the specified name
/// </summary>
Expand All @@ -15,21 +27,30 @@ public static class BranchCollectionExtensions
/// <param name="commit">The target commit.</param>
/// <param name="allowOverwrite">True to allow silent overwriting a potentially existing branch, false otherwise.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public static Branch Add(this BranchCollection branches, string name, Commit commit, bool allowOverwrite = false)
public static Branch Add(this BranchCollection branches, string name, Commit commit, bool allowOverwrite)
{
Ensure.ArgumentNotNull(commit, "commit");

return branches.Add(name, commit.Sha, allowOverwrite);
}

/// <summary>
/// Deletes the branch with the specified name.
/// </summary>
/// <param name="name">The name of the branch to delete.</param>
/// <param name="branches">The <see cref="BranchCollection"/> being worked with.</param>
public static void Remove(this BranchCollection branches, string name)
{
branches.Remove(name, false);
}

/// <summary>
/// Deletes the branch with the specified name.
/// </summary>
/// <param name="name">The name of the branch to delete.</param>
/// <param name="isRemote">True if the provided <paramref name="name"/> is the name of a remote branch, false otherwise.</param>
/// <param name="branches">The <see cref="BranchCollection"/> being worked with.</param>
public static void Remove(this BranchCollection branches, string name, bool isRemote = false)
public static void Remove(this BranchCollection branches, string name, bool isRemote)
{
Ensure.ArgumentNotNullOrEmptyString(name, "name");

Expand All @@ -45,6 +66,18 @@ public static void Remove(this BranchCollection branches, string name, bool isRe
branches.Remove(branch);
}

/// <summary>
/// Rename an existing local branch, using the default reflog message
/// </summary>
/// <param name="currentName">The current branch name.</param>
/// <param name="newName">The new name the existing branch should bear.</param>
/// <param name="branches">The <see cref="BranchCollection"/> being worked with.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public static Branch Rename(this BranchCollection branches, string currentName, string newName)
{
return branches.Rename(currentName, newName, false);
}

/// <summary>
/// Rename an existing local branch, using the default reflog message
/// </summary>
Expand All @@ -53,7 +86,7 @@ public static void Remove(this BranchCollection branches, string name, bool isRe
/// <param name="allowOverwrite">True to allow silent overwriting a potentially existing branch, false otherwise.</param>
/// <param name="branches">The <see cref="BranchCollection"/> being worked with.</param>
/// <returns>A new <see cref="Branch"/>.</returns>
public static Branch Rename(this BranchCollection branches, string currentName, string newName, bool allowOverwrite = false)
public static Branch Rename(this BranchCollection branches, string currentName, string newName, bool allowOverwrite)
{
Ensure.ArgumentNotNullOrEmptyString(currentName, "currentName");
Ensure.ArgumentNotNullOrEmptyString(newName, "newName");
Expand Down
46 changes: 43 additions & 3 deletions LibGit2Sharp/CommitRewriteInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,46 @@ public static CommitRewriteInfo From(Commit commit)
};
}

/// <summary>
/// Build a <see cref="CommitRewriteInfo"/> from the <see cref="Commit"/> passed in,
/// optionally overriding some of its properties
/// </summary>
/// <param name="commit">The <see cref="Commit"/> whose information is to be copied</param>
/// <param name="author">Optional override for the author</param>
/// <returns>A new <see cref="CommitRewriteInfo"/> object that matches the info for the
/// <paramref name="commit"/> with the optional parameters replaced..</returns>
public static CommitRewriteInfo From(Commit commit, Signature author)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd drop this one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a test - CanRewriteAuthorOfCommits - which uses this API. I've backed out of the change for now but if we want to 🔥 that test we can drop this.

{
return From(commit, author, null, null);
}

/// <summary>
/// Build a <see cref="CommitRewriteInfo"/> from the <see cref="Commit"/> passed in,
/// optionally overriding some of its properties
/// </summary>
/// <param name="commit">The <see cref="Commit"/> whose information is to be copied</param>
/// <param name="message">Optional override for the message</param>
/// <returns>A new <see cref="CommitRewriteInfo"/> object that matches the info for the
/// <paramref name="commit"/> with the optional parameters replaced..</returns>
public static CommitRewriteInfo From(Commit commit, string message)
{
return From(commit, null, null, message);
}

/// <summary>
/// Build a <see cref="CommitRewriteInfo"/> from the <see cref="Commit"/> passed in,
/// optionally overriding some of its properties
/// </summary>
/// <param name="commit">The <see cref="Commit"/> whose information is to be copied</param>
/// <param name="author">Optional override for the author</param>
/// <param name="committer">Optional override for the committer</param>
/// <returns>A new <see cref="CommitRewriteInfo"/> object that matches the info for the
/// <paramref name="commit"/> with the optional parameters replaced..</returns>
public static CommitRewriteInfo From(Commit commit, Signature author, Signature committer)
{
return From(commit, author, committer, null);
}

/// <summary>
/// Build a <see cref="CommitRewriteInfo"/> from the <see cref="Commit"/> passed in,
/// optionally overriding some of its properties
Expand All @@ -46,9 +86,9 @@ public static CommitRewriteInfo From(Commit commit)
/// <returns>A new <see cref="CommitRewriteInfo"/> object that matches the info for the
/// <paramref name="commit"/> with the optional parameters replaced..</returns>
public static CommitRewriteInfo From(Commit commit,
Signature author = null,
Signature committer = null,
string message = null)
Signature author,
Signature committer,
string message)
{
var cri = From(commit);
cri.Author = author ?? cri.Author;
Expand Down
Loading