I'm reviewing version 0.51 of the DiggiDig extension: https://addons.mozilla.org/en-US/firefox/addon/diggidig/, and found that it has String prototype extension in file options.js, line 219. This should have been picked up by the validator. These are the current results for that file: https://addons.mozilla.org/en-US/developers/addon/diggidig/file/134270/validation
I can't see the validation results on prod. Can you pastebin the output for me?
Here's the JSON output: http://pastebin.com/vcb5eELq
I'm going to say that this is the intended behavior. String prototypes (or any prototypes, for that matter, are treated the same way. There's no way right now to "traverse back" through the global scope to see where the JSPrototype object is referenced from, so any modification to it is a global variable overwrite error. As for the code not appearing in the snippet, that's because the assignment spans multiple lines. Since assignments are statements and not declarations, the "start" line (which we use to position the window for the snippet) is at the end of the assignment, rather than the beginning (where the String.prototype.diggiTrim bit is). There's really no workaround for that unless Spidermonkey changes the way it generates its parse tree, unfortunately. Unless there's something particular about this specific case that you think would be appropriate, I can't do a whole lot to make the warning easier to interpret.
It'd be more effective then to change the validation to a regular expression like (String|Number|Object).prototype.something = And then you can distinguish it from the other generic errors.
The problem there is that it would yield a second error since that kind of assignment is considered an overwrite of a critical global object. It could be marked as non-critical, but then we wouldn't get warnings/errors for things like this: var x = String; x.prototype.something = foo; or var x = String.prototype; x.something = foo; The regex would also have to consider the alternative means of assignment: String.prototype["something"] = foo; where "something" can be any expression.
Almost every single prototype extension case I've seen uses a pattern similar to comment #5. Since the current warnings are not being helpful, I think that adding the RE is an improvement. Extending it to cover String.prototype["something"] sounds good.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...
Whiteboard: [required amo-editors] → [ReviewTeam]
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.