Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a mutating return type operator to indicate whether a function modifies a property indirectly #29346

Closed
5 tasks done
dead-claudia opened this issue Jan 10, 2019 · 7 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@dead-claudia
Copy link

dead-claudia commented Jan 10, 2019

Search Terms

  • mutating function

Suggestion

I would like to see a return type operator T mutates this[A], U[B], ... to indicate that zero or more properties on types, like this or U above, are modified when the method is called. If either this[A] or U[B] is readonly, the method should be visible but uncallable, and if all depended-upon properties are writable, it should be both visible and callable. From the callee's perspective, it'd only see T.

Alternatively, you could implement it as a helper type Mutates<T, [{O: this, K: A}, {O: U, K: B}]>, but there are pitfalls I will explain here in a bit:

type Mutates<T, Deps extends {O: unknown, K: keyof this["O"]}[]> = {
	0: Deps extends [] ? T :
		((d: Deps) => void) extends ((first: infer A, ...rest: infer B) => void) ?
			{readonly [P in A["K"]]: unknown} extends A["O"] ? Mutates<T, B> :
			never :
		never
}[0]

The implementation above is complicated (I need the ability to intersect tuples' entries like I can union their entries via T[number]), but it's conceptually simple: foo(value: A): Mutates<B, this[number]> is equivalent to foo(value: A): this extends {readonly [P in number]: unknown} ? never : B.

The issue with this deriviation is that it doesn't prevent you from calling the method - it just prevents you from using the result. So if you were to add a theoretical uncallable primitive type to prevent even invoking the function, it'd work more like this:

type Mutates<T, Deps extends {O: unknown, K: keyof this["O"]}[]> = {
	0: Deps extends [] ? T :
		((d: Deps) => void) extends ((first: infer A, ...rest: infer B) => void) ?
			{readonly [P in A["K"]]: unknown} extends A["O"] ? Mutates<T, B> :
			uncallable :
		never
}[0]

This would change the desugaring to this, which is what I really want: foo(value: A): Mutates<B, this[number]> is equivalent to foo(value: A): this extends {readonly [P in number]: unknown} ? uncallable : B. However, it's pretty plainly obvious that this is a terrible idea to implement as a primitive type, which is why I proposed it as a new return type operator.

In terms of assignability, (value: A) => T mutating U[K] is assignable to (value: A) => T and (value: A) => T mutating SubtypeOfU[K], but not (value: Readonly<A>) => T, (value: Readonly<A>) => never, or (value: A) => T mutating SupertypeOfU[K]. Also, (value: A) => T and (value: Readonly<A>) => T are themselves assignable to (value: Readonly<A>) => T mutating U[K] for all U and K.

In addition to the above, I propose this should be generally inferred for TS functions, only required in type definitions.

Use Cases

This would enable you to patch the Array<T> type appropriately to allow the obvious type ReadonlyArray<T> = Readonly<Array<T>>. Conveniently, if you patch all the appropriate methods, you could even ensure it's covariant. But this wouldn't be the only area where it'd help, such as:

  • Readonly transactions, where you could define transaction(storeName: string, mode: "readonly"): Readonly<IDBTransaction> to enforce readonly-ness of that transaction at the type level.
  • Enforcing immutability around maps, sets, and the like.

Examples

interface Array<T> {
	splice(index: number, remove: number, ...replacements: T[]): T[] mutates this[number], this["length"]
	push(value: T): number mutates this[number], this["length"]
	pop(): T mutates this[number], this["length"]
}

// An internal marker symbol
type SetSym = unique symbol
interface Set<T> {
	[SetSym]: never
	add(value: T): this mutates this[SetSym], this["size"]
	delete(value: T): boolean mutates this[SetSym], this["size"]
	clear(): void mutates this[SetSym], this["size"]
}

// An internal marker symbol
type MapSym = unique symbol
interface Map<K, V> {
	[MapSym]: never
	set(key: K, value: V): this mutates this[MapSym], this["size"]
	delete(key: K): boolean mutates this[MapSym], this["size"]
	clear(): void mutates this[MapSym], this["size"]
}

// Little bit of setup code for the interesting stuff.
interface IDBTransaction {
	objectStore(name: string): this extends Readonly<IDBTransaction> ? Readonly<IDBObjectStore> : IDBObjectStore
}

// An internal marker symbol
type IDBObjectStoreSym = unique symbol
interface IDBObjectStore {
	[IDBObjectStoreSym]: never
	add(value: Value, key: Key): IDBRequest mutates this[IDBObjectStoreSym]
	clear(): IDBRequest mutates this[IDBObjectStoreSym]
	createIndex(name: string): IDBRequest mutates this[IDBObjectStoreSym]
	delete(key: Key | KeyRange): IDBRequest mutates this[IDBObjectStoreSym]
	deleteIndex(name: string): IDBRequest mutates this[IDBObjectStoreSym]
	put(value: Value, key: Key): IDBRequest mutates this[IDBObjectStoreSym]
	openCursor(): this extends Readonly<IDBObjectStore> ? IDBCursor : IDBCursorWithValue
	openKeyCursor(): IDBCursor
}

type IDBCursorSym = unique symbol
interface IDBCursor {
	[IDBCursorSym]: never
	// All the usual properties
	advance(count: number): void mutates this[IDBCursorSym]
	continue(key?: Key): void mutates this[IDBCursorSym]
	continuePrimaryKey(key: Key, primaryKey: Key): void mutates this[IDBCursorSym]
}
interface IDBCursorWithValue extends IDBCursor {
	delete(): void
	update(): void
}

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@weswigham weswigham added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jan 10, 2019
@weswigham
Copy link
Member

I've, personally, felt the need for tracing of certain effects (mutations included) before. The other effect we often see needed is weather an argument is immediately invoked or not. Effects are interesting because they'd force us to track refinements about localized symbol information alongside the types - and that information is only useful when those symbols are in scope (eg, if a function type that mutates an internal variable flows into an external scope, nothing can witness the mutation, so the refinement is no longer needed). Correctly tracking this information would require encoding a lot of symbolic information into types and do correspondingly more analysis during flow control analysis.

@dead-claudia
Copy link
Author

Yeah, this is much more specific than that, a fairly small (but useful) subset of effects. It doesn't really reify it into a type, just warns callers. Conceptually, it's closer to Swift's mutating keyword than Eff's eff types. It's like a small expansion of readonly, just to methods.

The explicit dependency here you have to list is so, in theory, you don't need to gensym any types or type dependencies into existence.

Correctly tracking this information would require encoding a lot of symbolic information into types and do correspondingly more analysis during flow control analysis.

Yeah, definitely. But the flow control analysis for my proposal I don't believe should have to change significantly. It's just a few main things:

  1. Dropping effects on local variables that go out of scope.
  2. Merging effects on all in-scope variables when going out of each branch.
  3. Accumulating effects each step in the way for further refinement.

I wouldn't be against full effect types and such, if you all are willing to commit to it, though... 😉

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Jan 11, 2019

declarations like this are cool and all, how are you going to use them (enforce them, constrain your code because of them)?

note aside, i have a hard feeling on the proposed syntax when you say: Mutates<...> and then mutates..., i like how flow tackles this meta-type problems with special $... constructs, typescript needs something like this desperately

lastly and sadly, what are the chances if we still don't have essentials like HKT's, etc

@dead-claudia
Copy link
Author

@Aleksey-Bykov

declarations like this are cool and all, how are you going to use them (enforce them, constrain your code because of them)?

That's a linter problem, not a language problem. The key here is that Readonly<T> would be more like a type-level immutable Object.freeze and properties' readonly is treated more like property descriptors' configurable slot.

note aside, i have a hard feeling on the proposed syntax when you say: Mutates<...> and then mutates..., [...]

I used the generic pseudo-type to illustrate a possible alternative solution, and then showed where the pitfalls were in it.

[...] i like how flow tackles this meta-type problems with special $... constructs, typescript needs something like this desperately

IIUC the TS design team has been very cautious to not introduce those. Instead, they prefer to introduce either clear syntax for it, like keyof T ↔ Flow's $KeyType<T> or the minimal primitives required to create such a helper, like type InstanceType<T> = T extends new (...args: any[]) => infer R ? R : never ↔ Flow's $InstanceType<T>. This is the better route, and although it takes longer to design, it comes out far more flexible and it results in a lot less compiler work down the road.

lastly and sadly, what are the chances if we still don't have essentials like HKT's, etc

This has 0% to do with HKTs, and in fact, most of the use cases I've encountered are with concrete types and interfaces, not abstract ones.

@dead-claudia
Copy link
Author

@weswigham Here's a userland alternative that might work: add a Mutates<T, KS> generic type that mandates T to not have read-only keys KS. This would be used on the parameter side, so the above type definitions would change to this:

type Mutates<T, KS extends keyof T> =
	T extends {readonly [P in KS]: T[P]} ? never : T

interface Array<T> {
	splice(this: Mutates<this, number | "length">, index: number, remove: number, ...replacements: T[]): T[]
	push(this: Mutates<this, number | "length">, value: T): number
	pop(this: Mutates<this, number | "length">): T
}

interface Set<T> {
	add(this: Mutates<this, "size">, value: T): this
	delete(this: Mutates<this, "size">, value: T): boolean
	clear(this: Mutates<this, "size">): void
}

interface Map<K, V> {
	set(this: Mutates<this, "size">, key: K, value: V): this
	delete(this: Mutates<this, "size">, key: K): boolean
	clear(this: Mutates<this, "size">): void
}

// Little bit of setup code for the interesting stuff.
// An internal marker symbol
type IDBTransactionSym = unique symbol
interface IDBTransaction {
	[IDBTransactionSym]: never
	objectStore(name: string): this extends Readonly<IDBTransaction> ? Readonly<IDBObjectStore> : IDBObjectStore
}

// An internal marker symbol
type IDBObjectStoreSym = unique symbol
interface IDBObjectStore {
	[IDBObjectStoreSym]: never
	add(this: Mutates<this, IDBObjectStoreSym>, value: Value, key: Key): IDBRequest
	clear(this: Mutates<this, IDBObjectStoreSym>): IDBRequest
	createIndex(this: Mutates<this, IDBObjectStoreSym>, name: string): IDBRequest
	delete(this: Mutates<this, IDBObjectStoreSym>, key: Key | KeyRange): IDBRequest
	deleteIndex(this: Mutates<this, IDBObjectStoreSym>, name: string): IDBRequest
	put(this: Mutates<this, IDBObjectStoreSym>, value: Value, key: Key): IDBRequest
	openCursor(): this extends Readonly<IDBObjectStore> ? IDBCursor : IDBCursorWithValue
	openKeyCursor(): IDBCursor
}

interface IDBCursor {
	// All the usual properties
	advance(this: Mutates<this, "key" | "primaryKey">, count: number): void
	continue(this: Mutates<this, "key" | "primaryKey">, key?: Key): void
	continuePrimaryKey(this: Mutates<this, "key" | "primaryKey">, key: Key, primaryKey: Key): void
}

interface IDBCursorWithValue extends IDBCursor {
	value: any
	delete(this: Mutates<this, "key" | "primaryKey" | "value">): void
	update(this: Mutates<this, "key" | "primaryKey" | "value">, value: any): void
}

After looking at that, I feel I like that better, and maybe that could just be a built-in type with some things in the docs to note how this could be used.

@dead-claudia
Copy link
Author

dead-claudia commented Jan 12, 2019

@weswigham Related comment...is that a bug? In either case, it's blocking this.

@dead-claudia
Copy link
Author

Closing in favor of #29435 + a potential future effects suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants