588243 - notice single quotes in package.json and complain usefully

Status: RESOLVED WORKSFORME

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Brian Warner [:warner :bwarner]]

I heard on the jetpack call today that using single-quotes in a package.json file (which is not correct according to a strict JSON parser, but is occasionally accepted by some parsers, possibly including the browser) causes subtle problems. This bug is to have the "cfx xpi" call notice such illegal JSON and complain about it usefully, rather than allowing it to be copied into the XPI and causing even harder-to-understand problems later.

Comment 1

[Julián Ceballos]

I was thinking, just three lines of code could resolve that problem, just replacing the single quotes and assuring that the package.json would be like the standard says, but notice to the user that he has to write their package.json with double quotes.

The code could be something like this:

if str(data).find("'")!=-1:
        open(path, 'w').write(str(data).replace("'",'"'))
        data = open(path, 'r').read()

And in the stdout show something like this:
"You've wrote your package.json with single quotes, we've changed for you, but is so important let you know that the json specifications says that you have to write it with double quotes."

Obviously something shortest XD jeje!.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(automatic reprioritization of 1.0 bugs)

Comment 4

[Brian Warner [:warner :bwarner]]

Hm, single quotes are valid inside strings: imagine an addon with:

 "description": "This addon is awesome. It's the best addon ever.",

So I don't think a simple ("'" in data) or automatic-replacement is likely to work.

I just checked, and Python's simplejson.loads() function rejects single-quotes. (it isn't pretty: you get a messy cuddlefish.packaging.MalformedJsonFileError, sometimes with a line number, sometimes not). So we don't need to worry about some bad JSON getting past the SDK and into the XPI.

I'm going to close this, assuming that the error message is good enough to lead developers to fixing their JSON. If it turns out that's not the case, feel free to re-open it. I'm not sure how we'd improve the error message: simplejson.loads() doesn't provide a distinctive exception when the problem is a single-quote instead of a double-quote. The best idea that comes to mind right now is to catch MalformedJsonFileError and print a generic "here's all the ways your JSON might be wrong" message, which could include single-quotes along with the other possibilities (missing comma, extra comma, unquoted property name, etc).