Skip to content
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

fix(semver): replace semver library with translation from node-semver #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MikeDombo
Copy link
Member

@MikeDombo MikeDombo commented Mar 18, 2022

Translated from
npm/node-semver@cb1ca1d.

Translated SemVer, Range, Comparator, and their associated tests.

Issue #, if available:

Description of changes:
Translation of node-semver in order to fix deficiencies in the com.vdurmont.semver4j library.

Bumped component common library version to 2.2.0-SNAPSHOT.

Why is this change necessary:

How was this change tested:
Ported tests from node-semver which are passing. Existing component recipe tests either passed or were minorly updated to pass.

Any additional information or context required to review the change:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

return compareIdentifiers(this.patch, other.patch);
}

public int comparePre(SemVer other) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The cyclomatic complexity of this method is 13. By comparison, 98% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test.

We recommend that you simplify this method or break it into multiple methods. For example, consider extracting the code block on lines 149-155 into a separate method.


// preminor will bump the version up to the next minor release, and immediately
// down to pre-release. premajor and prepatch work the same way.
public SemVer inc(String release, String identifier) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The cyclomatic complexity of this method is 18. By comparison, 99% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.

if (isX(M)) {
ret = "";
} else if (isX(m)) {
ret = String.format(">=%s.0.0 <%s.0.0-0", M, Integer.parseInt(M) + 1);
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

This code uses '%s' to format int: '+' expression. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 327. Consider formatting this data with '%d' instead.

.collect(Collectors.joining(" "));
}

private static String replaceXRange(String s) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The cyclomatic complexity of this method is 19. By comparison, 99% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.

.collect(Collectors.joining(" "));
}

static String replaceCaret(String c) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The cyclomatic complexity of this method is 11. By comparison, 98% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.

return Integer.compare(a, b);
}

public static int compareIdentifiers(String a, String b) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The cyclomatic complexity of this method is 11. By comparison, 98% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test. We recommend that you simplify this method or break it into multiple methods.

}

protected String format() {
String v = String.format("%s.%s.%s", this.major, this.minor, this.patch);
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

This code uses '%s' to format int: patch, major, minor. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 102. Consider formatting this data with '%d' instead.

if (isX(tM)) {
to = "";
} else if (isX(tm)) {
to = String.format("<%s.0.0-0", Integer.parseInt(tM) + 1);
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

This code uses '%s' to format int: '+' expression. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 434. Consider formatting this data with '%d' instead.


ret = String.format("%s%s.%s.%s%s", gtlt, M, m, p, pr);
} else if (xm) {
ret = String.format(">=%s.0.0%s <%s.0.0-0", M, pr, Integer.parseInt(M) + 1);
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

This code uses '%s' to format int: '+' expression. This is a potential locale-sensitive handling issue. It might cause errors in the handling and processing of the statement at line: 292. Consider formatting this data with '%d' instead.

return a.compareTo(b) <= 0;
}

public boolean intersects (Comparator comp) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The cyclomatic complexity of this method is 22. By comparison, 99% of the methods in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the method has a high number of decisions and it can make the logic difficult to understand and test.

We recommend that you simplify this method or break it into multiple methods. For example, consider extracting the code block on lines 125-135 into a separate method.

Requirement versionRequirement;
Range versionRequirement;

Choose a reason for hiding this comment

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

Are Requirements and Ranges equivalent?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's the semver term for what the other library called a requirement.

Copy link
Member Author

@MikeDombo MikeDombo Mar 18, 2022

Choose a reason for hiding this comment

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

I'd recommend downloading this branch and then writing test code to play around with it and check whatever you think is needed. Just a code review with this volume of code is unlikely to find anything.

Translated from
npm/node-semver@cb1ca1d.

Translated SemVer, Range, Comparator, and their associated tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants