GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Quick fix for no-implicit-any errors to add explicit type annotation by mhegazy · Pull Request #14786 · microsoft/TypeScript

Conversation

@mhegazy

@mhegazy Mohamed Hegazy (mhegazy) commented Mar 22, 2017

Copy link
Copy Markdown
Contributor

Add a new quick fix for no-implicit-any errors for variables, parameters, and members. The fix tries to "infer" the type from assignments to the symbol in question, and serializes the type in an explicit type annotation addressing the no-implicit-any error.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aluanhaddad, mohsen1, HerringtonDarkholme, steve8708, arkaev, OliverJAsh, SlurpTheo, BrianMikinski, aminpaks, and mickdekkers reacted with hooray emoji DanielRosenwasser, aluanhaddad, remojansen, localvoid, steve8708, arkaev, BrianMikinski, and mickdekkers reacted with heart emoji
All reactions
  • 🎉 10 reactions
  • ❤️ 8 reactions
Mohamed Hegazy (mhegazy) added 29 commits February 25, 2017 21:05
@mhegazy
Infer from usage quick fix
a4ce5e6
@mhegazy
Change full function singature
d79fbc0
@mhegazy
Add property/element access support
69a4b8c
@mhegazy
Fix a few issues
33467cf
@mhegazy
Merge branch 'master' into inferFromUsage
ab39813
@mhegazy
Some cleanup
d87eae9
@mhegazy
Expose getArrayType and getPromiseType
a18a950
@mhegazy
Switch to collecting all usage before infering
bea4591
@mhegazy
Infer array and promise type arguments
55ffe2d
@mhegazy
Handel enums in binary operators
b82dc8d
@mhegazy
consolidate usage of addCandidateTypes
7a52545
@mhegazy
Handel rest paramters
8088415
@mhegazy
Properly handel += and + inference for numbers and strings
0413165
@mhegazy
Add print quickfixes debug helper
ea483a7
@mhegazy
Add rest param tests
ea344b2
@mhegazy
Add optional paramter tests
a1efc1a
@mhegazy
Handel set accessors
ff15dc6
@mhegazy
Support getters
e2e43d1
@mhegazy
Support no implicit any error for variable at use site
2929af4
@mhegazy
Support properties
b395057
@mhegazy
Only offer quick fix if an infered type other than any is available
15eb7de
@mhegazy
Rename functions
2a9032b
@mhegazy
Move to a separate namespace
057016b
@mhegazy
Check cancellation token
32aa07d
@mhegazy
Cleanup
5da31bb
@mhegazy
Check for accesibile symbols where serializing types
dc431e4
@mhegazy
Remove JS support
561cfe6
@mhegazy
Reorganize functions
002e8ad
@mhegazy
Mark APIs as internal
3822d8d
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Mar 22, 2017
@mhegazy
Merge remote-tracking branch 'origin/master' into inferFromUsage
c3071a8
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) changed the title Quick fix for no-implicit-any errors to ad explicit type annotation Quick fix for no-implicit-any errors to add explicit type annotation Mar 22, 2017
@aluanhaddad

Aluan Haddad (aluanhaddad) commented Mar 31, 2017

Copy link
Copy Markdown
Contributor

I am delighted by this. Thank you Mohamed Hegazy (@mhegazy)!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Apr 17, 2017
Control flow analysis based on operators applied to an argument #15196
Closed
Add a refactoring that annotates an implicit or explicit any with a best guess type inferred from its use sites #13243
Open
@mhegazy
Merge branch 'master' into inferFromUsage
a12dc45
DanielRosenwasser
Daniel Rosenwasser (DanielRosenwasser) requested changes May 9, 2017
View reviewed changes
Comment thread src/compiler/checker.ts
},
getApparentType,

getUnionType,

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) May 9, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe makeUnionType or createUnionType? This works too though.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts
case Diagnostics.Variable_0_implicitly_has_type_1_in_some_locations_where_its_type_cannot_be_determined.code:
return getCodeActionForVariableDeclaration(<PropertyDeclaration | PropertySignature | VariableDeclaration>token.parent);
case Diagnostics.Variable_0_implicitly_has_an_1_type.code:
return getCodeActionForVariableUsage(<Identifier>token);

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) May 9, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there should be assertions when you cast based on the error code you're matching on.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 7, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We asserted a few lines earlier that it has to be an identifier, DotDotDotToken, PublicKeyword, ProtectedKeyword, or ReadonlyKeyword. since this is a variable declaration error, we know it can not be anything that is not an identifier (i.e. binding pattern).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
case Diagnostics.Variable_0_implicitly_has_an_1_type.code:
return getCodeActionForVariableUsage(<Identifier>token);

// Paramtert declarations

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) May 9, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameter

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 7, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
return typeString;
}

function getParameterIndexInList(parameter: ParameterDeclaration, list: NodeArray<ParameterDeclaration>) {

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) May 9, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this just indexOf?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts
for (const child of node.getChildren(sourcefile)) {
if (child.kind === kind) return child;
}
}

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) May 9, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicit return undefined

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 7, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts
return -1;
}

function getFirstChildOfKind(node: Node, sourcefile: SourceFile, kind: SyntaxKind) {

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) May 9, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a findChildOfKind which is nice and slow and allocates the lamdba you were trying to avoid here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated

// Property declarations
case Diagnostics.Member_0_implicitly_has_an_1_type.code:
return getCodeActionForVariableDeclaration(<VariableDeclaration>token.parent);

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) May 9, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever a time that you won't have a symbol that you can't call this directly? What about in the case of a computed property?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 7, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We filter these earlier on. it can only be identifier for a var declarations

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Daniel Rosenwasser (DanielRosenwasser) added 2 commits August 1, 2017 14:14
@DanielRosenwasser
Merge branch 'master' into inferFromUsage
791bb4a 
Conflicts:
	src/compiler/checker.ts
	src/compiler/diagnosticMessages.json
	src/compiler/types.ts
	src/services/tsconfig.json
@DanielRosenwasser
Removed conflict markers.
0047ae3
DanielRosenwasser
Daniel Rosenwasser (DanielRosenwasser) reviewed Aug 1, 2017
View reviewed changes
Comment thread src/compiler/checker.ts
getUndefinedType: () => undefinedType,
getNullType: () => nullType,
getESSymbolType: () => esSymbolType,
getNeverType: () => neverType,

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Aug 1, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a concern, we have multiple never types internally. I feel somewhat uneasy about exposing it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 7, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made them all them internal for now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@DovydasNavickas

Dovydas Navickas (DovydasNavickas) commented Sep 6, 2017

Copy link
Copy Markdown

Add new lines to the end of files?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Daniel Rosenwasser (DanielRosenwasser) added 4 commits September 19, 2017 00:43
@DanielRosenwasser
Update 'createSymbol' to use '__String'.
5a15e93
@DanielRosenwasser
Fixed most problems relating to '__String' and 'includeJsDocComments'…
9daaffa 
… in the fix itself.
@DanielRosenwasser
Addressed most API changes.
b5e94a7
@DanielRosenwasser
Merge remote-tracking branch 'origin/master' into inferFromUsage
3b49776
@msftgits Microsoft GitHub User (msftgits) removed the cla-not-required label Sep 26, 2017
Mohamed Hegazy (mhegazy) added 6 commits October 5, 2017 16:17
@mhegazy
Merge branch 'master' into inferFromUsage
3f488ce
@mhegazy
Make all helpers internal
6bdaf8a
@mhegazy
Use a diffrent writer and not the built-in single line write
25f8aa6
@mhegazy
Infer types for all parameters in a parameter list instead of one at …
761bd7b 
…a time
@mhegazy
Accept baselines
ffa8272
@mhegazy
Code review commments
5abd72d
@mhegazy

Mohamed Hegazy (mhegazy) commented Oct 7, 2017

Copy link
Copy Markdown
Contributor Author

Daniel Rosenwasser (@DanielRosenwasser) any more comments?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sandersn
Nathan Shively-Sanders (sandersn) requested changes Oct 10, 2017
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions about inference, plus I noticed some typos.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
case Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation.code:
return isSetAccessor(containingFunction) ? getCodeActionForSetAccessor(containingFunction) : undefined;

// Property declarations

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks redundant with the very first case of this switch.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. removed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts
errorCodes: [
// Variable declarations
Diagnostics.Variable_0_implicitly_has_type_1_in_some_locations_where_its_type_cannot_be_determined.code,
Diagnostics.Variable_0_implicitly_has_an_1_type.code,

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this error maps to getCodeActionForVariableUsage, so I think it is actually should be commented with // variable uses

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts
return undefined;
}

const types = inferTypeForParametersFromUsage(containingFunction) ||

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it's either from usage from calls or from intra-function usages, but not both? Why have inferences from call sites win?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do the function body first we are guranteed to manifacture something that the makes the funciton happy, but it usually will be too general. giving usage precedence allows us to get more specific types.. e.g.:

function length(a) { return a.length; }

length([1,2,3]);

inferring from function body will give a a type { length: any}. which is accurate, but rarely what the user would write manually.

inferring from the call site gives us number[] which is more like it..

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@masaeedu Asad Saeeduddin (masaeedu) Nov 8, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you should infer <T extends { length }>(a: T) => ... instead. In combination with return type inference, this should yield <T extends { length }>(a: T) => T["length"], which will work correctly when applied to arrays (or indeed anything with a length property).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Nov 8, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that.. the types get unwieldy very fast.. and you have this structural type that has length, push, splice, with an element that is has kind, instead of just Node[]. the call sites provided the more user-identifiable name.. though the first one is more correct.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
const types = inferTypeForParametersFromUsage(containingFunction) ||
map(containingFunction.parameters, p => isIdentifier(p.name) && inferTypeForVariableFromUsage(p.name));

if (types) {

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be easier to read both nested blocks as early returns (if (!types) return undefined and if (!textChanges) return undefined.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated

if (types) {
const textChanges = reduceLeft(containingFunction.parameters, (memo, parameter, index) => {
if (types[index] && !parameter.type && !parameter.initializer) {

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

containingFunction.parameters.length === types.length, right?
If so, I don't think we need the efficiency of reduce and lazy initialisation of memo. It would be easier to read as a zip followed by filtering out undefined:
zipWith(containingFunction.parameters, types, (param, type) => ...).filter(change => change !== undefined)

(zipWith is in core.ts so it might need to be moved into the public API?)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
}

if (usageContext.callContexts) {
for (const callConext of usageContext.callContexts) {

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo:callContext

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts
return checker.createSignature(/*declaration*/ undefined, /*typeParameters*/ undefined, /*thisParameter*/ undefined, parameters, returnType, /*typePredicate*/ undefined, callContext.argumentTypes.length, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false);
}

function inferTypeFromContext(node: Expression, checker: TypeChecker, usageContext: UsageContext): void {

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this before getTypeFromUsageContext since it's used before?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, moved get* after infer*

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
return;
}
else {
const indextType = checker.getTypeAtLocation(parent);

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably is a typo for indexType ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
function getSignatureFromCallContext(callContext: CallContext, checker: TypeChecker): Signature {
const parameters: Symbol[] = [];
for (let i = 0; i < callContext.argumentTypes.length; i++) {
const symbol = checker.createSymbol(SymbolFlags.FunctionScopedVariable, escapeLeadingUnderscores(`p${i}`));

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this arg${i} to match our other names for unnamed parameters.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/codefixes/inferFromUsage.ts Outdated
else if (usageContext.properties && hasCallContext(usageContext.properties.get("push" as __String))) {
return checker.createArrayType(getParameterTypeFromCallContexts(0, usageContext.properties.get("push" as __String).callContexts, /*isRestParameter*/ false, checker));
}
else if (usageContext.properties || usageContext.callContexts || usageContext.constructContexts || usageContext.numberIndexContext || usageContext.stringIndexContext) {

@sandersn Nathan Shively-Sanders (sandersn) Oct 10, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if you combine object types made up of the synthetic properties (et al) with the candidate types? Are the candidate types strictly better than the synthetic object type?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 11, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think so.. a user-defined type seems more like what you would put on a function, e.g. Node or Symbol, and not {kind: SyntaxKind} or {flags: SymbolFlags} to use an example of our code base.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mhegazy
Respond to code review comments
1e39d48
sandersn
Nathan Shively-Sanders (sandersn) approved these changes Oct 12, 2017
View reviewed changes
@mhegazy
Mohamed Hegazy (mhegazy) merged commit 4487917 into master Oct 12, 2017
@mhegazy
Mohamed Hegazy (mhegazy) deleted the inferFromUsage branch October 12, 2017 17:15
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Nov 8, 2017
Discussion: Parameter type inference from function body #17715
Closed
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) DanielRosenwasser requested changes
@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes
+1 more reviewer
@masaeedu Asad Saeeduddin (masaeedu) masaeedu left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@mhegazy @aluanhaddad @DovydasNavickas @sandersn @DanielRosenwasser @masaeedu @msftclas @msftgits
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.