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.

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

Jump to bottom

fix(maven): Use consistent precision for extended ranges #6623

Merged
merged 8 commits into from Jul 1, 2020
Merged

fix(maven): Use consistent precision for extended ranges #6623

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cba9558
fix(maven): Use consistent precision for extended ranges
Jun 28, 2020
d28cdda
Increment left value if needed
Jun 28, 2020
6501923
Refactor
Jun 28, 2020
2eedb3a
More fixes and coverage
Jun 28, 2020
e10b8b3
Merge branch 'master' of github.com:renovatebot/renovate into fix/pre…
Jun 28, 2020
c0de13d
Fix extending behaviour
Jun 29, 2020
bac1ee3
Fix lint
Jun 29, 2020
9e846b6
Fix coverage
Jun 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 49 additions & 4 deletions lib/versioning/maven/compare.ts
View file
Open in desktop
Expand Up @@ -113,7 +113,7 @@ const zeroToken: NumberToken = {
isTransition: false,
};

function tokenize(versionStr: string): Token[] {
function tokenize(versionStr: string, preserveMinorZeroes = false): Token[] {
let buf: Token[] = [];
let result: Token[] = [];
let leadingZero = true;
Expand All @@ -126,7 +126,7 @@ function tokenize(versionStr: string): Token[] {
leadingZero = false;
result = result.concat(buf);
buf = [];
} else if (leadingZero) {
} else if (leadingZero || preserveMinorZeroes) {
result = result.concat(buf);
buf = [];
}
Expand Down Expand Up @@ -444,6 +444,41 @@ function rangeToStr(fullRange: Range[]): string | null {
return intervals.join(',');
}

function tokensToStr(tokens: Token[]): string {
return tokens.reduce((result, token, idx) => {
const prefix = token.prefix === PREFIX_DOT ? '.' : '-';
return `${result}${idx !== 0 && token.val !== '' ? prefix : ''}${
token.val
}`;
}, '');
}

function coerceRangeValue(prev: string, next: string): string {
const prevTokens = tokenize(prev, true);
const nextTokens = tokenize(next, true);
const resultTokens = nextTokens.slice(0, prevTokens.length);
const align = Math.max(0, prevTokens.length - nextTokens.length);
for (let i = 0; i < align; i += 1) {
const tokenIdx = prevTokens.length + i - 1;
const token = prevTokens[tokenIdx];
resultTokens.push(nullFor(token));
}
return tokensToStr(resultTokens);
}

function incrementRangeValue(value: string): string {
if (value === null) {
return value;
}
const tokens = tokenize(value);
const lastToken = tokens[tokens.length - 1];
if (typeof lastToken.val === 'number') {
lastToken.val += 1;
return tokensToStr(tokens);
}
return value;
}

function autoExtendMavenRange(
currentRepresentation: string,
newValue: string
Expand Down Expand Up @@ -484,9 +519,19 @@ function autoExtendMavenRange(
}
const interval = range[nearestIntervalIdx];
if (interval.rightValue !== null) {
interval.rightValue = newValue;
const newLeftValue = incrementRangeValue(interval.leftValue);
const newRightValue = coerceRangeValue(interval.rightValue, newValue);

if (
newLeftValue &&
newLeftValue === interval.rightValue &&
newLeftValue !== newRightValue
) {
interval.leftValue = newLeftValue;
}
interval.rightValue = newRightValue;
} else {
interval.leftValue = newValue;
interval.leftValue = coerceRangeValue(interval.leftValue, newValue);
}
if (interval.leftValue && interval.rightValue) {
if (compare(interval.leftValue, interval.rightValue) !== 1) {
Expand Down
23 changes: 19 additions & 4 deletions lib/versioning/maven/index.spec.ts
View file
Open in desktop
Expand Up @@ -251,15 +251,30 @@ describe('versioning/maven/compare', () => {
['],1.0]', '2.0', '],2.0]'],
['(,1.0)', '2.0', '(,2.0)'],
['],1.0[', '2.0', '],2.0['],
['[1.0,1.2],[1.3,1.5)', '1.2.4', '[1.0,1.2.4],[1.3,1.5)'],
['[1.0,1.2],[1.3,1.5[', '1.2.4', '[1.0,1.2.4],[1.3,1.5['],
['[1.0,1.2.3],[1.3,1.5)', '1.2.4', '[1.0,1.2.4],[1.3,1.5)'],
['[1.0,1.2.3],[1.3,1.5[', '1.2.4', '[1.0,1.2.4],[1.3,1.5['],
['[1.2.3,)', '1.2.4', '[1.2.4,)'],
['[1.2.3,[', '1.2.4', '[1.2.4,['],
['[1.2.3,]', '1.2.4', '[1.2.3,]'], // invalid range
['[0.21,0.22)', '0.20.21', '[0.21,0.22)'],
['[0.21,0.22)', '0.21.1', '[0.21,0.22)'],
['[0.21,0.22)', '0.22.1', '[0.21,0.22.1)'],
['[0.21,0.22)', '0.23', '[0.21,0.23)'],
['[0.21,0.22.0)', '0.22.1', '[0.21,0.22.1)'],
['[0.21,0.22)', '0.23', '[0.22,0.23)'],

['[1.8,1.9)', '1.9.0.1', '[1.8,1.9)'],
['[1.8a,1.9)', '1.9.0.1', '[1.8a,1.9)'],
['[1.8,1.9.0)', '1.9.0.1', '[1.8,1.9.0)'],
['[1.8,1.9.0.0)', '1.9.0.1', '[1.8,1.9.0.1)'],
['[1.8,1.9.0.0)', '1.10.1', '[1.8,1.10.1.0)'],

['[1.8,1.9)', '1.9.1', '[1.8,1.9)'],
['[1.8,1.9)', '1.10.0', '[1.9,1.10)'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

If new value is 1.10.0 then shouldn't it be [1.10,1.11) ?

Copy link
Collaborator Author

@zharinov zharinov Jun 29, 2020 •
edited

Choose a reason for hiding this comment

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

Agree, it would look better in this context. What upgrade for [1.8, 1.9] and 1.10.0 would be better:[1.8, 1.10] or [1.9, 1.10]? Btw probably we need to think about range strategies other than auto for maven.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be 1.11)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, take a look again, I've made a slightly different implementation

['[1.8,1.9)', '1.10.1', '[1.9,1.10)'],

['(,1.0.0]', '2.0.0', '(,2.0.0]'],
['(,1.0]', '2.0.0', '(,2.0]'],
['(,1]', '2.0.0', '(,2]'],
['(,1.0.0-foobar]', '2.0.0', '(,2.0.0]'],
];
sample.forEach(([oldRepr, newValue, newRepr]) => {
expect(autoExtendMavenRange(oldRepr, newValue)).toEqual(newRepr);
Expand Down