Bug 145954 – Web Inspector: Improve some cases of "Object?" Type Annotations

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
Improve some cases of "Object?" Type Annotations.

If a type is (Object | NullOrUndefined) and no other primitives just show an "Object?" annotation instead of a nice "LeastCommonAncestor?" annotation.

* TEST
<script>
class Person {};

function personOrNullAndUndefined(person) {} // Person?
personOrNullAndUndefined(new Person);
personOrNullAndUndefined(null);
personOrNullAndUndefined(undefined);

function personOrStringOrNullAndUndefined(thing) {} // Object?
personOrStringOrNullAndUndefined(new Person);
personOrStringOrNullAndUndefined(null);
personOrStringOrNullAndUndefined(undefined);
personOrStringOrNullAndUndefined("string");

function personOrBooleanOrNullAndUndefined(thing) {} // (many)
personOrStringOrNullAndUndefined(new Person);
personOrStringOrNullAndUndefined(null);
personOrStringOrNullAndUndefined(undefined);
personOrStringOrNullAndUndefined(false);
</script>

[Joseph Pecoraro]

> function personOrBooleanOrNullAndUndefined(thing) {} // (many)
> personOrStringOrNullAndUndefined(new Person);
> personOrStringOrNullAndUndefined(null);
> personOrStringOrNullAndUndefined(undefined);
> personOrStringOrNullAndUndefined(false);

Err, these should obviously be calling the right function. heh

[Joseph Pecoraro]

Created attachment 254848 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Noticed this by looking at PropertyPath's constructor. Parameters with:

    "PropertyPath | Undefined" showed "Object?" expected "PropertyPath?"
    "RemoteObject | Null"      showed "Object?" expected "RemoteObject?"

After the path this looks as expected.

[Saam Barati]

Looks good to me. 
I'd r+ if I could.

[Saam Barati]

Comment on attachment 254848 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=254848&action=review

> Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:135
> +        if (this._typeDescription.leastCommonAncestor) {

This is a good catch.

> Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:138
>              if (typeSet.isContainedIn(WebInspector.TypeSet.TypeBit.Object | WebInspector.TypeSet.NullOrUndefinedTypeBits))

This branch doesn't even make sense given the above branch above will preclude things that primitive types from coming through.

[Saam Barati]

(In reply to comment #5)
> Comment on attachment 254848 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254848&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:135
> > +        if (this._typeDescription.leastCommonAncestor) {
> 
> This is a good catch.
> 
> > Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:138
> >              if (typeSet.isContainedIn(WebInspector.TypeSet.TypeBit.Object | WebInspector.TypeSet.NullOrUndefinedTypeBits))
> 
> This branch doesn't even make sense given the above branch above will
> preclude things that primitive types from coming through.
Err: This should be past tense. It didn't make sense before. It makes sense now.

[WebKit Commit Bot]

Comment on attachment 254848 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 254848

Committed r185562: <http://trac.webkit.org/changeset/185562>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.