This repository was archived by the owner on Jun 5, 2020. It is now read-only.
Fix type errors #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Array and Object were often written as array and object, causing warnings such as: ./buffer.js:32: WARNING - Bad type annotation. Unknown type object * @type {object.<string,*>} ^ Fix the warnings by correcting the capitalization. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
The intent of these annotations seems clear, fix the typos. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
The type requires the buffer namespace in order to be resolved. Add it where missing. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
From source inspection and usage, it's clear that Buffer doesn't extend Array (for example, Buffer has no pop method). Remove this annotation (which also fixes the type incompatibility of the slice method). Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
cluster is actually an instance of the private Cluster type, which extends EventEmitter. Therefore, annotate cluster as being of type EventEmitter. Note: This code is now not runnable, since assigning to cluster would require either reference to the events namespace (which isn't imported) or leaving it null which would fail for subsequent property accesses. But that's fine, since externs aren't run. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
According to the docs (and code) this argument is optional. This also fixes the warning about non-optional arguments following optional arguments. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Although it has the same type as the global require function, the global require function isn't a type (constructor). So update the type of module.require to match it. This fixes the following warning: ./core.js:76: WARNING - Bad type annotation. Unknown type require * @type {require} ^ Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Update the annotations to match their definitions in node 0.10. Also, stream (the namespace) is not a class, use stream.Stream instead as appropriate. This commit doesn't fix the multiple inheritance/mixin issue with Duplex needing to extend from both Readable and Writable. This may require defining some interfaces which can be implemented. This commit also doesn't touch ReadableStream and WritableStream. I'm not sure of the purpose of these classes and since they are heavily used in other namespaces, I'll leave them alone for now. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
All of the functions which take a time argument accept either numbers (as seconds since the epoch) or a Date. Add the Date type to arguments where it was missing. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
The properties of Stat with Date type were being assigned numbers, which caused compile warnings. Remove the unnecessary assignment. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Replace the http.ServerRequest and http.ClientResponse classes with the IncomingMessage class, as is done in recent versions of node. Add the missing members to the prototype. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Lots of refactoring to work around the fact that Closure Compiler doesn't have proper support for mixins. Ensure that @extends meets the requirement that X instanceof Y if X @extends Y. Add copies of the annotations for all mixin methods and aliased functions. Also add missing annotations for pbkdf2. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
These members are all inherited from net.Server and don't need to be redefined for tls.Server (and the listen definition was causing type warnings). Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
I'm not sure if this used to be added by node, but I can't find it
anywhere in the docs or code in recent versions. It was also causing
the following warning (due to a conflict with util.inspect):
./util.js:77: WARNING - assignment to property inspect of Object.<string,*>
found : function (*, boolean=, number=, boolean=): string
required: function (this:Object): ?
util.inspect = function(object, showHidden, depth, colors) {};
^
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Until we decide what to do with WritableStream, fix it to avoid producing any warnings. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
The side-effect of this method is copying data into the target buffer given as an argument. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
The side-effectfulness of these functions is determined based on whether the callback function has side-effects. Presumably it does (what's the point of the callback otherwise?). So remove the @nosideeffects annotation. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Owner
|
Looks like you are better in writing externs than I am, so I added you as a collaborator :). You should now be able to commit / merge changes directly. |
Collaborator
Author
|
Hah. Guess that's an indication I've spent too much time trying to figure this thing out. ;) Thanks! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a follow-up to the last pull request, I've gone ahead and fixed the type errors in the non-contrib modules.
I think this resolves most of the issues, although there are a few things that I didn't touch that you may want to take a look at:
ReadableStreamandWritableStreamwere. I couldn't find any references in the node documentation. So I mostly left them alone.stream.Duplexcurrently doesn't inherit fromstream.Writable. This needs to be fixed, but due to Issue 250 and lack of support for mixins, this won't be easy.streammodule may break code using really old versions of node. This namespace has changed a lot and I'm not very familiar with the old interfaces. Although it looks safe, I'm sure there are some corner cases.Cheers,
Kevin