Skip to content

strict mode for parsing numbers #6

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

strict mode for parsing numbers #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
87d15c6
strict mode for parsing numbers
russaa Sep 4, 2018
604707f
added test for invalid 'NaN' input
russaa Sep 6, 2018
9464034
simplified normalizeNumber function as suggested by @jedmao
russaa Sep 6, 2018
707532f
simplified tryParseStrict function as suggested by @jedmao
russaa Sep 6, 2018
8cffa15
removed return-type declaration from functions
russaa Sep 6, 2018
1a379c6
refactor: return NULL in normalizeNumber() instead of throwing error
russaa Sep 6, 2018
8f6cdab
extended test cases
russaa Sep 6, 2018
3321734
added support for numbers with leading zeros in strict mode
russaa Sep 6, 2018
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
187 changes: 129 additions & 58 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,103 +6,172 @@ const cssResolutionUnits: string[] = require('css-resolution-units');
const cssFrequencyUnits: string[] = require('css-frequency-units');
const cssTimeUnits: string[] = require('css-time-units');

const parseOptions = [, {strict: false}, {strict: true}];

import CssDimension from './';

test('throws when a dot should be followed by a number', (t) => {
t.throws(
() => new CssDimension('12.'),
/The dot should be followed by a number/,
);
parseOptions.forEach((options) => {
t.throws(
() => new CssDimension('12.', options),
/The dot should be followed by a number/,
);
});
});

test('throws when more than one leading +/- is provided', (t) => {
t.throws(
() => new CssDimension('+-12.2'),
/Only one leading \+\/- is allowed/,
);
parseOptions.forEach((options) => {
t.throws(
() => new CssDimension('+-12.2', options),
/Only one leading \+\/- is allowed/,
);
});
});

test('throws when more than one dot is provided', (t) => {
t.throws(
() => new CssDimension('12.1.1'),
/Only one dot is allowed/,
);
parseOptions.forEach((options) => {
t.throws(
() => new CssDimension('12.1.1', options),
/Only one dot is allowed/,
);
});
});

test('throws when an invalid unit of "foo" is provided', (t) => {
t.throws(
() => new CssDimension('12foo'),
/Invalid unit: foo/,
);
parseOptions.forEach((options) => {
t.throws(
() => new CssDimension('12foo', options),
/Invalid unit: foo/,
);
});
});

test('throws when an invalid number of "foo42" is provided', (t) => {
test('throws in strict mode when an invalid number of "foo42" is provided', (t) => {
parseOptions.forEach((options) => {
t.throws(
() => new CssDimension('foo42', options),
/Invalid number: foo/,
);
});
});

const nonNumbers = [
'+.NaN%',
'NaN%',
];

test('throws in when non-number input with "NaN" is provided', (t) => {
parseOptions.forEach((options) => {
nonNumbers.forEach((nonNumber) => {
t.throws(
() => new CssDimension(nonNumber, options),
new RegExp('Invalid number: ' + nonNumber.replace('+', '\\+').replace(/%$/, '')),
);
});
});
});

const strictInvalidENotationNumbers = [
'23e.07',
'23e0.7',
'23e-.07',
'23e+0.07',
];

test('throws in strict mode when a number with invalid e-notation is provided', (t) => {
strictInvalidENotationNumbers.forEach((invalidNumber) => {
t.throws(
() => new CssDimension(invalidNumber, {strict: true}),
new RegExp('Invalid number: ' + invalidNumber.replace('+', '\\+')),
);
});
});

test('throws in strict when an invalid number of "35sdfs75rem" is provided', (t) => {
t.throws(
() => new CssDimension('foo42'),
/Invalid number: foo/,
() => new CssDimension('35sdfs75rem', {strict: true}),
/Invalid number: 35sdfs75/,
);
});

test('parse result is instance of CssDimension', (t) => {
t.is(
CssDimension.parse('42%') instanceof CssDimension,
true,
);
parseOptions.forEach((options) => {
t.is(
CssDimension.parse('42%', options) instanceof CssDimension,
true,
);
});
});

test('returns the numeric value with .value', (t) => {
t.true(typeof new CssDimension('42%').value === 'number');
parseOptions.forEach((options) => {
t.true(typeof new CssDimension('42%', options).value === 'number');
});
});

test('adding a number yields a concatenated string', (t) => {
t.is(
(CssDimension.parse('42%') as any) + 3,
'42%3',
);
parseOptions.forEach((options) => {
t.is(
(CssDimension.parse('42%', options) as any) + 3,
'42%3',
);
});
});

test('stringifies a percent', (t) => {
t.is(
new CssDimension('42%') + 'foo',
'42%foo',
);
parseOptions.forEach((options) => {
t.is(
new CssDimension('42%', options) + 'foo',
'42%foo',
);
});
});

test('stringifies a number', (t) => {
t.is(
new CssDimension('42') + 'foo',
'42foo',
);
parseOptions.forEach((options) => {
t.is(
new CssDimension('42', options) + 'foo',
'42foo',
);
});
});

const validNumbers = {
'+0.0': 0,
'-.1': -0.1,
'-0.0': -0,
'-3.4e-2': -0.034,
'-456.8': -456.8,
'.60': 0.6,
'0.0': 0,
'000289.6800': 289.68,
'068': 68,
'10E-3': 0.01,
'10e3': 10000,
'12': 12,
'4.01': 4.01,
'540': 540,
'89.3000': 89.3,
};

test('number conversion', (t) => {
const unit = '%';
Object.keys(validNumbers).forEach((rawNumber) => {
const num = validNumbers[rawNumber];

const d1 = CssDimension.parse(rawNumber);
let msg = 'parses ' + rawNumber;
t.is(d1.type, 'number', msg);
t.is(d1.value, num, msg);
t.is(d1.unit, undefined, msg);

const d2 = CssDimension.parse(rawNumber + unit);
msg += unit;
t.is(d2.type, 'percentage', msg);
t.is(d2.value, num, msg);
t.is(d2.unit, unit, msg);
parseOptions.forEach((options) => {
Object.keys(validNumbers).forEach((rawNumber) => {
const num = validNumbers[rawNumber];

const d1 = CssDimension.parse(rawNumber, options);
let msg = 'parses ' + rawNumber;
t.is(d1.type, 'number', msg);
t.is(d1.value, num, msg);
t.is(d1.unit, undefined, msg);

const d2 = CssDimension.parse(rawNumber + unit, options);
msg += unit;
t.is(d2.type, 'percentage', msg);
t.is(d2.value, num, msg);
t.is(d2.unit, unit, msg);
});
});
});

Expand Down Expand Up @@ -130,14 +199,16 @@ test('units and unit types', (t) => {
},
].forEach((unitDef) => {
unitDef.list.forEach((unit) => {
Object.keys(validNumbers).forEach((rawNumber) => {
const num = validNumbers[rawNumber];

const d1 = CssDimension.parse(rawNumber + unit);
const msg = 'parses ' + rawNumber + unit;
t.is(d1.type, unitDef.type, msg);
t.is(d1.value, num, msg);
t.is(d1.unit, unit, msg);
parseOptions.forEach((options) => {
Object.keys(validNumbers).forEach((rawNumber) => {
const num = validNumbers[rawNumber];

const d1 = CssDimension.parse(rawNumber + unit, options);
const msg = 'parses ' + rawNumber + unit;
t.is(d1.type, unitDef.type, msg);
t.is(d1.value, num, msg);
t.is(d1.unit, unit, msg);
});
});
});
});
Expand Down
78 changes: 72 additions & 6 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,46 @@ const cssResolutionUnits: string[] = require('css-resolution-units');
const cssFrequencyUnits: string[] = require('css-frequency-units');
const cssTimeUnits: string[] = require('css-time-units');

const numberPrefixPattern = /^(\+|-)?(\.)?\d/;
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this so far away from the implementation? Also, I can remove the trailing \d and all the tests still pass.

Copy link
Author

Choose a reason for hiding this comment

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

yes, there needs to be be test added for this:
without this you could enter "NaN%" and it would falsely validate it a number


export interface IOptions {
strict?: boolean;
}

export default class CssDimension {

public static parse(value: string) {
return new CssDimension(value);
public static parse(value: string, options?: IOptions) {
return new CssDimension(value, options);
}

public type: string;
public value: number;
public unit: string;

constructor(value: string) {
constructor(value: string, options?: IOptions) {

this.validateNumber(value);
this.validateSign(value);
this.validateDots(value);

const strict = !!(options && options.strict);

if (/%$/.test(value)) {
this.type = 'percentage';
this.value = tryParseFloat(value);
this.value = tryParseNumber(value.substring(0, value.length - 1), strict);
Copy link
Owner

Choose a reason for hiding this comment

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

What's going on here with the substring?

Copy link
Author

Choose a reason for hiding this comment

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

it removes the percent sign so that (potentially) only the number itself will be parsed (removal of the percent sign could be omitted in non-strict mode)

this.unit = '%';
return;
}

const unit = parseUnit(value);
if (!unit) {
this.type = 'number';
this.value = tryParseFloat(value);
this.value = tryParseNumber(value, strict);
return;
}

this.type = unitToType(unit);
this.value = tryParseFloat(value.substr(0, value.length - unit.length));
this.value = tryParseNumber(value.substr(0, value.length - unit.length), strict);
this.unit = unit;
}

Expand Down Expand Up @@ -74,6 +83,10 @@ function countDots(value: string) {
return m ? m.length : 0;
}

function tryParseNumber(value: string, strict: boolean) {
return strict ? tryParseStrict(value) : tryParseFloat(value);
}

function tryParseFloat(value: string) {
const result = parseFloat(value);
if (isNaN(result)) {
Expand All @@ -82,6 +95,59 @@ function tryParseFloat(value: string) {
return result;
}

function normalizeNumber(value: string, allowDot: boolean = true) {
value = value[0] === '0' ? value.replace(/^0+(\d)/, '$1') : value;
const match = numberPrefixPattern.exec(value);
if (!match) {
return null;
}
const [, sign, dot] = match;
if (sign === '+') {
value = value.substr(1);
}
if (dot) {
if (!allowDot) {
return null;
}
if (sign === '-') {
value = '-0' + value.substr(1);
} else {
value = '0' + value;
}
}
return (dot || countDots(value))
? value.replace(/\.?0+$/, '')
: value;
}

function tryParseStrict(value: string) {
const nval = normalizeNumber(value);
if (!nval) {
throw new Error(`Invalid number: ${value}`);
}
const result = parseFloat(nval);
if (result.toString() !== nval && !verifyZero(value) && !verifyENotation(value)) {
throw new Error(`Invalid number: ${value}`);
}
return result;
}

function verifyZero(value: string) {
return /^[-+]?0\.0+$/.test(value);
Copy link
Owner

Choose a reason for hiding this comment

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

The following works w/o breaking tests:

return parseFloat(value) === 0;

Copy link
Author

Choose a reason for hiding this comment

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

I guess so, but parseFloat() will always ignore trailing non-number parts in the string -- I have not thought too hard about this (in this instance), but it may be that there is such a case that it would fail here (i.e. falsely claim a non-valid input as number)

Copy link
Owner

@jednano jednano Sep 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, this is the discussion I was trying to spark here. Those other cases might be worth testing. You could also consider Math.abs(value) === 0.

}

function verifyENotation(value: string) {
const m = /e/i.exec(value);
if (!m || m.index === value.length - 1) {
return false;
}
const nval = normalizeNumber(value.substring(m.index + 1), false);
if (!nval) {
throw new Error(`Invalid number: ${value}`);
}
return parseInt(nval, 10).toString() === nval;
}

const units = cssAngleUnits.concat(
cssFrequencyUnits,
cssLengthUnits,
Expand Down