String.toInt produces "Ok NaN" for some inputs

[TobiDo]

String.toIntresults to Ok NaN for the arguments "+", "-", "0x" respectively.

See Ellie: https://ellie-app.com/mjSxjQHdka1/1

Here is the implementation of toInt from Elm/Kernel/String.js:

function _String_toInt(s)
{
	var len = s.length;

	// if empty
	if (len === 0)
	{
		return _String_intErr(s);
	}

	// if hex
	var c = s[0];
	if (c === '0' && s[1] === 'x')
	{
		for (var i = 2; i < len; ++i)
		{
			var c = s[i];
			if (('0' <= c && c <= '9') || ('A' <= c && c <= 'F') || ('a' <= c && c <= 'f'))
			{
				continue;
			}
			return _String_intErr(s);
		}
		return __Result_Ok(parseInt(s, 16));
	}

	// is decimal
	if (c > '9' || (c < '0' && ((c !== '-' && c !== '+') || len === 1)))
	{
		return _String_intErr(s);
	}
	for (var i = 1; i < len; ++i)
	{
		var c = s[i];
		if (c < '0' || '9' < c)
		{
			return _String_intErr(s);
		}
	}

	return __Result_Ok(parseInt(s, 10));
}

I think the best solution would be to check whether parseInt returned NaN before returning an Ok-Result. One could, of course, also handle the three aforementioned cases explicitly.

[process-bot]

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[process-bot]

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[zwilias]

+ and - are covered by #834; 0x is one I hadn't heard about; but also seems fixable. That toInt implementation is starting to become pretty gnarly.

[zwilias]

+ and - are covered by #834; 0x is one I hadn't heard about; but also seems fixable. That toInt implementation is starting to become pretty gnarly.

[evancz]

@zwilias, it sounds like some of this is resolved. If there is any left, open a new one with the relevant SSCCEs so I can figure out what is actually broken at the moment!

[evancz]

@zwilias, it sounds like some of this is resolved. If there is any left, open a new one with the relevant SSCCEs so I can figure out what is actually broken at the moment!

[zwilias]

https://github.com/elm-lang/core/issues/949 SSCCE/overview created 👍

[zwilias]

https://github.com/elm-lang/core/issues/949 SSCCE/overview created 👍