speced / bikeshed

:bike: A preprocessor for anyone writing specifications that converts source files into actual specs.
https://speced.github.io/bikeshed
Creative Commons Zero v1.0 Universal
1.1k stars 199 forks source link

Error/warning if webidl and dfn try to define the same method? #913

Open mkruisselbrink opened 7 years ago

mkruisselbrink commented 7 years ago

I'm having some issue with the definition of the FileReader (and FileReaderSync) readAsText methods in the FileAPI spec. These are defined in webidl as:

interface FileReader {
  void readAsText(Blob blob, optional DOMString label);
};

And defined in prose as <dfn method for=FileReader id="dfn-readAsText">readAsText(blob, optional label)</dfn>.

This used to work fine, but with current bikeshed this result in link errors for references trying to reference {{FileReader/readAsText()}}, since apparently now this results in two separate definitions for the same thing. It would be very helpful if bikeshed could give a warning/error for the duplicate definition rather than only give an error when trying to reference this. Also not entirely sure if this is supposed to work or not?

Removing the optional from the dfn resolves the error (and I'm happy to do so), but this used to work even with the optional there.

Related, while trying to debug this it seemed that the presence of an id attribute also changed if I got a link error or not. Removing the id attribute, the link error goes away (although the dfn and the webidl definition still wouldn't be linked).

And another related issue, the -l option is kind of useless for debugging things like this, since apparently it doesn't support any kind of link shorthands, so even with the option there is no way to get line number information for link errors.

tabatkins commented 7 years ago

Yeah, the WebIDL recognizer code is getting technically better, but in a way that happens to make this markup example stop working. ^_^

Previously, the method-canonicalizer would do the correct thing if your dfn text was the full WebIDL signature, with types and everything; if it wasn't, it would give up and just barf out all the signatures it knew of for that method name. This worked for you - that markup snippet isn't a full WebIDL signature, so you got everything.

Now it has some additional modes, which make it more correct in general - it also explicitly recognizes "just a list of argument names", and spits out the correct set of signatures. But that's not what your snippet has - it's has the optional keyword, so it's halfway in-between "full signature" and "just argnames". It looks like this now causes the method canonicalizer to just spit out the literal text, and that means the IDL block's definition doesn't match up, and you get multiple definitions of the method.

The fix for now is to do one or the other in your manual definition - either use a full WebIDL signature, or use just argnames. I'll need to look into how to recognize this situation and flag it as an error.