Skip to content

Fix how output and generic types are checked in the breaking change tool #7273

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.

Sign up for GitHub

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 1 commit into from
Sep 25, 2018
Merged

Fix how output and generic types are checked in the breaking change tool #7273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 19 additions & 14 deletions tools/StaticAnalysis/BreakingChangeAnalyzer/CmdletMetadataHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void CompareCmdletMetadata(
// If the cmdlet cannot be found, log an issue
else
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: oldCmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedCmdlet,
Expand Down Expand Up @@ -145,7 +145,7 @@ private void CheckForRemovedCmdletAlias(
// If the alias cannot be found, log an issue
if (!aliasSet.Contains(oldAlias))
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: oldCmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedCmdletAlias,
Expand All @@ -171,7 +171,7 @@ private void CheckForRemovedSupportsShouldProcess(
// If the old cmdlet implements SupportsShouldProcess and the new cmdlet does not, log an issue
if (oldCmdlet.SupportsShouldProcess && !newCmdlet.SupportsShouldProcess)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: oldCmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedShouldProcess,
Expand All @@ -194,7 +194,7 @@ private void CheckForRemovedSupportsPaging(
// If the old cmdlet implements SupportsPaging and the new cmdlet does not, log an issue
if (oldCmdlet.SupportsPaging && !newCmdlet.SupportsPaging)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: oldCmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedPaging,
Expand Down Expand Up @@ -239,17 +239,22 @@ private void CheckForChangedOutputType(

_typeMetadataHelper.CheckOutputType(oldCmdlet, oldOutput.Type, newOutputType, issueLogger);
}
// If the output cannot be found, log an issue
// If the output cannot be found by name, check if the old output can be mapped
// to any of the new output types
else
{
issueLogger.LogBreakingChangeIssue(
cmdlet: oldCmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedOutputType,
description: string.Format(Properties.Resources.ChangedOutputTypeDescription,
oldCmdlet.Name, oldOutput.Type.Name),
remediation: string.Format(Properties.Resources.ChangedOutputTypeRemediation,
oldCmdlet.Name, oldOutput.Type.Name));
var foundOutput = outputDictionary.Values.Any(o => _typeMetadataHelper.CompareTypeMetadata(oldCmdlet, oldOutput.Type, o, null));
if (!foundOutput)
{
issueLogger?.LogBreakingChangeIssue(
cmdlet: oldCmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedOutputType,
description: string.Format(Properties.Resources.ChangedOutputTypeDescription,
oldCmdlet.Name, oldOutput.Type.Name),
remediation: string.Format(Properties.Resources.ChangedOutputTypeRemediation,
oldCmdlet.Name, oldOutput.Type.Name));
}
}
}
}
Expand Down Expand Up @@ -302,7 +307,7 @@ private void CheckDefaultParameterName(
// If the parameter cannot be found, log an issue
if (!parameterDictionary.ContainsKey(oldParameter.ParameterMetadata.Name))
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: oldCmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangeDefaultParameter,
Expand Down
16 changes: 8 additions & 8 deletions tools/StaticAnalysis/BreakingChangeAnalyzer/ParameterMetadataHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void CompareParameterMetadata(
// If the parameter cannot be found, log an issue
else
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedParameter,
Expand Down Expand Up @@ -149,7 +149,7 @@ private void CheckForRemovedParameterAlias(
// If the alias cannot be found, log an issue
if (!aliasSet.Contains(oldAlias))
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedParameterAlias,
Expand Down Expand Up @@ -187,7 +187,7 @@ private void CheckParameterValidationSets(
// one in the new assembly, log an issue
if (oldValidateSet.Count == 0 && newValidateSet.Count > 0)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.AddedValidateSet,
Expand Down Expand Up @@ -215,7 +215,7 @@ private void CheckParameterValidationSets(
// If the value cannot be found, log an issue
if (!valueSet.Contains(oldValue))
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedValidateSetValue,
Expand Down Expand Up @@ -246,7 +246,7 @@ private void CheckParameterValidateRange(
// If the old parameter had no validation range, but the new parameter does, log an issue
if (oldParameter.ValidateRangeMin == null && oldParameter.ValidateRangeMax == null)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.AddedValidateRange ,
Expand All @@ -260,7 +260,7 @@ private void CheckParameterValidateRange(
// If the minimum value of the range has increased, log an issue
if (oldParameter.ValidateRangeMin < newParameter.ValidateRangeMin)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedValidateRangeMinimum,
Expand All @@ -273,7 +273,7 @@ private void CheckParameterValidateRange(
// If the maximum value of the range has decreased, log an issue
if (oldParameter.ValidateRangeMax > newParameter.ValidateRangeMax)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedValidateRangeMaximum,
Expand Down Expand Up @@ -303,7 +303,7 @@ private void CheckForValidateNotNullOrEmpty(
// old assembly, but has it in the new assembly, log an issue
if (!oldParameter.ValidateNotNullOrEmpty && newParameter.ValidateNotNullOrEmpty)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.AddedValidateNotNullOrEmpty,
Expand Down
2 changes: 1 addition & 1 deletion tools/StaticAnalysis/BreakingChangeAnalyzer/ParameterSetMetadataHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void CompareParameterSetMetadata(

if (!foundMatch)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedParameterSet,
Expand Down
79 changes: 33 additions & 46 deletions tools/StaticAnalysis/BreakingChangeAnalyzer/TypeMetadataHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@ public TypeMetadataHelper(
/// <param name="oldType">The type metadata from the old (serialized) assembly.</param>
/// <param name="newType">The type metadata from the new assembly.</param>
/// <param name="issueLogger">ReportLogger that will keep track of issues found.</param>
public void CompareTypeMetadata(
public bool CompareTypeMetadata(
CmdletMetadata cmdlet,
TypeMetadata oldType,
TypeMetadata newType,
ReportLogger<BreakingChangeIssue> issueLogger)
{
var result = true;

// For each property in the old assembly type, find the corresponding
// property in the new assembly type
foreach (var oldProperty in oldType.Properties.Keys)
Expand Down Expand Up @@ -109,29 +111,33 @@ public void CompareTypeMetadata(
else
{
// If the type of the property has been changed, log an issue
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedPropertyType,
description: string.Format(Properties.Resources.ChangedPropertyTypeDescription,
oldProperty, oldType.Name, oldPropertyType, newPropertyType),
remediation: string.Format(Properties.Resources.ChangedPropertyTypeRemediation,
oldProperty, oldPropertyType));
result = false;
}
}
else
{
// If the property has been removed, log an issue
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.RemovedProperty,
description: string.Format(Properties.Resources.RemovedPropertyDescription,
oldProperty, oldType.Name),
remediation: string.Format(Properties.Resources.RemovedPropertyRemediation,
oldProperty, oldType.Name));
result = false;
}
}

return result;
}

public void CompareMethodSignatures(
Expand Down Expand Up @@ -179,7 +185,7 @@ public void CompareMethodSignatures(

if (!found)
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: 0,
Expand Down Expand Up @@ -228,7 +234,7 @@ public void CheckParameterType(
// If the types are different, log an issue
if (!oldTypeMetadata.Name.Equals(newTypeMetadata.Name, StringComparison.OrdinalIgnoreCase))
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedParameterType,
Expand Down Expand Up @@ -320,7 +326,7 @@ private bool IsElementType(
// If the element type has changed, log an issue
else
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: problemId,
Expand Down Expand Up @@ -366,20 +372,34 @@ private bool IsGenericType(
var oldArgument = oldTypeMetadata.GenericTypeArguments[idx];
var newArgument = newTypeMetadata.GenericTypeArguments[idx];

if (CheckGenericTypeArguments(cmdlet, oldArgument, newArgument, target, issueLogger))
var oldElementType = _oldTypeDictionary[oldArgument];
var newElementType = _newTypeDictionary[newArgument];
if (string.Equals(oldArgument, newArgument, StringComparison.OrdinalIgnoreCase))
{
// If we have not previously seen this generic type argument,
// run this method on the type
if (!_typeSet.Contains(oldArgument))
{
_typeSet.Add(oldArgument);

var oldElementType = _oldTypeDictionary[oldArgument];
var newElementType = _newTypeDictionary[newArgument];

CompareTypeMetadata(cmdlet, oldElementType, newElementType, issueLogger);
}
}
else
{
// If the generic type arguments have changed, throw a specific exception for generics
if (!CompareTypeMetadata(cmdlet, oldElementType, newElementType, issueLogger))
{
// If the generic type arguments aren't the same, log an issue
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedGenericTypeArgument,
description: string.Format(Properties.Resources.ChangedGenericTypeArgumentDescription,
target, oldArgument, newArgument),
remediation: string.Format(Properties.Resources.ChangedGenericTypeArgumentRemediation,
target, oldArgument));
}
}
}
}
}
Expand Down Expand Up @@ -412,7 +432,7 @@ private bool HasSameGenericType(
}

// Otherwise, log an issue and return false
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedGenericType,
Expand Down Expand Up @@ -447,7 +467,7 @@ private bool HasSameGenericArgumentSize(
}

// Otherwise, log an issue and return false
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.DifferentGenericTypeArgumentSize,
Expand All @@ -459,38 +479,5 @@ private bool HasSameGenericArgumentSize(

return false;
}

/// <summary>
/// Check if the arguments of a generic type are the same. If they are not, log an issue.
/// </summary>
/// <param name="cmdlet">The cmdlet metadata currently being checked.</param>
/// <param name="oldArgument">The old argument from the generic.</param>
/// <param name="newArgument">The new argument from the generic</param>
/// <param name="target">Target of the generic type breaking change.</param>
/// <param name="issueLogger">ReportLogger that will keep track of issues found.</param>
private bool CheckGenericTypeArguments(
CmdletMetadata cmdlet,
string oldArgument,
string newArgument,
string target,
ReportLogger<BreakingChangeIssue> issueLogger)
{
if (oldArgument.Equals(newArgument, StringComparison.OrdinalIgnoreCase))
{
return true;
}

// If the generic type arguments aren't the same, log an issue
issueLogger.LogBreakingChangeIssue(
cmdlet: cmdlet,
severity: 0,
problemId: ProblemIds.BreakingChangeProblemId.ChangedGenericTypeArgument,
description: string.Format(Properties.Resources.ChangedGenericTypeArgumentDescription,
target, oldArgument, newArgument),
remediation: string.Format(Properties.Resources.ChangedGenericTypeArgumentRemediation,
target, oldArgument));

return false;
}
}
}
3 changes: 1 addition & 2 deletions tools/StaticAnalysis/StaticAnalysis.Test/BreakingChangeAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,7 @@ public void ChangeOutputType()

xunitOutput.WriteLine(output);

Assert.Equal(1, testReport.ProblemIdList.Count);
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(BreakingChangeProblemId.ChangedOutputType)).SingleOrDefault<int>().Equals(BreakingChangeProblemId.ChangedOutputType));
Assert.Equal(0, testReport.ProblemIdList.Count);
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion tools/VersionController/Models/VersionMetadataHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private void CheckBreakingChangesInTypes(

if (!newTypeMetadataDictionary.ContainsKey(type))
{
issueLogger.LogBreakingChangeIssue(
issueLogger?.LogBreakingChangeIssue(
cmdlet: cmdletMetadata,
severity: 0,
problemId: 0,
Expand Down