Skip to content

Type hinting #116

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.

Sign up for GitHub

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

Open
krisavi wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Type hinting #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44eb9ed
Type hinting
krisavi Nov 18, 2019
7d2c689
Fix the test of automatic function signature
krisavi Nov 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions server/src/models/token.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ export class Token implements TokenBase {
public readonly end: Position;
public readonly uri: string;

public ref: Maybe<Token>;

/**
* What symbol tracker is this token tied too
*/
Expand All @@ -32,6 +34,7 @@ export class Token implements TokenBase {
this.start = start;
Copy link
Owner

@jonnyboyC jonnyboyC Nov 19, 2019

Choose a reason for hiding this comment

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

Since we only set this in the scanner I think we should just update the constructor and make ref readonly

constructor(
  type: TokenType,
  lexeme: string,
  literal: any,
  start: Position,
  end: Position,
  uri: string,
  ref?: Token) {
    // ...
}

this.end = end;
this.uri = uri;
this.ref = undefined;
this.tracker = undefined;
}

Expand All @@ -57,6 +60,10 @@ export class Token implements TokenBase {
};
}

public get getRef(): Token | undefined {
Copy link
Owner

@jonnyboyC jonnyboyC Nov 19, 2019

Choose a reason for hiding this comment

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

Probably don't need this getter here since this.ref already is available.

return this.ref;
}

/**
* Convert the token to a human readable string
*/
Expand Down
1 change: 1 addition & 0 deletions server/src/models/tokentypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export enum TokenType {
// whitespace
whiteSpace, commentLine,
region, endRegion,
type, returns,

plus, minus, multi, div, power,
not, and, or, true, false,
Expand Down
18 changes: 15 additions & 3 deletions server/src/parser/models/declare.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Stmt, Block } from './stmt';
import { IExpr, IStmtVisitor, ScopeKind, NodeDataBuilder } from '../types';
import { ITypeHint, IExpr, IStmtVisitor, ScopeKind, NodeDataBuilder } from '../types';
import { TokenType } from '../../models/tokentypes';
import { empty, unWrap } from '../../utilities/typeGuards';
import { Range, Position } from 'vscode-languageserver';
Expand Down Expand Up @@ -103,13 +103,15 @@ export class Var extends Decl {
public readonly toIs: Token;
public readonly value: IExpr;
public readonly scope: Scope;
public readonly typeHint?: ITypeHint;
Copy link
Owner

@jonnyboyC jonnyboyC Nov 19, 2019

Choose a reason for hiding this comment

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

Since we're applying typeHint to each declaration type I think we should move it up to the Decl super class and just pass typeHint into super in each case.

Copy link
Author

@krisavi krisavi Nov 19, 2019

Choose a reason for hiding this comment

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

I started with only parameters.
Later added other ones.

constructor is using builder for subclasses, but not for superclass. Seems to be few issues just moving to to superclass.


constructor(builder: NodeDataBuilder<Var>) {
super();
this.identifier = unWrap(builder.identifier);
this.toIs = unWrap(builder.toIs);
this.value = unWrap(builder.value);
this.scope = unWrap(builder.scope);
this.typeHint = builder.typeHint;
}

public toLines(): string[] {
Expand Down Expand Up @@ -148,6 +150,7 @@ export class Lock extends Decl {
public readonly to: Token;
public readonly value: IExpr;
public readonly scope?: Scope;
public readonly typeHint?: ITypeHint;

constructor(builder: NodeDataBuilder<Lock>) {
super();
Expand All @@ -156,6 +159,7 @@ export class Lock extends Decl {
this.to = unWrap(builder.to);
this.value = unWrap(builder.value);
this.scope = builder.scope;
this.typeHint = builder.typeHint;
}

public toLines(): string[] {
Expand Down Expand Up @@ -205,13 +209,15 @@ export class Func extends Decl {
public readonly identifier: Token;
public readonly block: Block;
public readonly scope?: Scope;
public readonly typeHint?: ITypeHint;

constructor(builder: NodeDataBuilder<Func>) {
super();
this.functionToken = unWrap(builder.functionToken);
this.identifier = unWrap(builder.identifier);
this.block = unWrap(builder.block);
this.scope = builder.scope;
this.typeHint = builder.typeHint;
}

public toLines(): string[] {
Expand Down Expand Up @@ -254,13 +260,15 @@ export class Param extends Decl {
public readonly requiredParameters: Parameter[];
public readonly optionalParameters: DefaultParam[];
public readonly scope?: Scope;
public readonly typeHint?: ITypeHint;

constructor(builder: NodeDataBuilder<Param>) {
super();
this.parameterToken = unWrap(builder.parameterToken);
this.requiredParameters = unWrap(builder.requiredParameters);
this.optionalParameters = unWrap(builder.optionalParameters);
this.scope = builder.scope;
this.typeHint = builder.typeHint;
}

public toLines(): string[] {
Expand Down Expand Up @@ -330,7 +338,10 @@ export class Param extends Decl {
}

export class Parameter extends NodeBase {
constructor(public readonly identifier: Token) {
constructor(
public readonly identifier: Token,
public readonly typeHint?: Maybe<string>,
) {
super();
}

Expand Down Expand Up @@ -360,8 +371,9 @@ export class DefaultParam extends Parameter {
identifier: Token,
public readonly toIs: Token,
public readonly value: IExpr,
typeHint?: Maybe<string>,
) {
super(identifier);
super(identifier, typeHint);
}

public toLines(): string[] {
Expand Down
28 changes: 23 additions & 5 deletions server/src/parser/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { TokenType, isValidIdentifier } from '../models/tokentypes';
import {
IParseError,
IExpr,
ITypeHint,
IStmt,
INodeResult,
RunStmtType,
Expand Down Expand Up @@ -42,6 +43,7 @@ export class Parser {
private tokens: Token[];
private current: number;
private runStmts: RunStmtType[];
private types: Token[];

private readonly andBind = this.and.bind(this);
private readonly eqaulityBind = this.equality.bind(this);
Expand All @@ -57,13 +59,15 @@ export class Parser {
tokens: Token[],
logger: ILogger = mockLogger,
tracer: ITracer = mockTracer,
types: Token[] = [],
) {
this.uri = uri;
this.tokens = tokens.concat(this.eof(tokens));
this.current = 0;
this.runStmts = [];
this.logger = logger;
this.tracer = tracer;
this.types = types;
}

// parse tokens
Expand All @@ -75,6 +79,9 @@ export class Parser {
this.logger.info(
`Parsing started for ${file} with ${this.tokens.length} tokens.`,
);
this.logger.info(
`Parsing started for ${file} with ${this.types.length} tokens.`,
Copy link
Owner

@jonnyboyC jonnyboyC Nov 19, 2019

Choose a reason for hiding this comment

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

maybe we could have something like Parsing started for ${file} with ${this.types.length} type hints.

);

const statements: Stmt.Stmt[] = [];
const parseDiagnostics: Diagnostic[] = [];
Expand Down Expand Up @@ -242,9 +249,10 @@ export class Parser {
}

// parse function declaration
private declareFunction(scope?: Decl.Scope): INodeResult<Decl.Func> {
private declareFunction(scope?: Decl.Scope, typeHint?: Maybe<ITypeHint>): INodeResult<Decl.Func> {
Copy link
Owner

@jonnyboyC jonnyboyC Nov 19, 2019

Choose a reason for hiding this comment

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

It doesn't appear anything is passed for typeHint here or in the other declare functions below. I think it would be good to remove them for now if this was temporary code.

const builder: NodeDataBuilder<Decl.Func> = {
scope,
typeHint,
functionToken: undefined,
identifier: undefined,
block: undefined,
Expand All @@ -261,6 +269,8 @@ export class Parser {
if (this.matchToken(TokenType.curlyOpen)) {
const block = this.block();
builder.block = block.value;

builder.typeHint = this.types.find(x => x.ref === builder.identifier)?.literal;

this.matchToken(TokenType.period);
return nodeResult(new Decl.Func(builder), block.errors);
Expand Down Expand Up @@ -308,7 +318,8 @@ export class Parser {
Decl.Parameter,
);

parameters.push(new Decl.Parameter(identifer));
const typeHint = this.types.find(x => x.ref === identifer);
parameters.push(new Decl.Parameter(identifer, typeHint?.literal));
} while (this.matchToken(TokenType.comma));

return parameters;
Expand All @@ -333,8 +344,9 @@ export class Parser {
[TokenType.to, TokenType.is],
);
const valueResult = this.expression();
const typeHint = this.types.find(x => x.ref === identifer);
defaultParameters.push(
new Decl.DefaultParam(identifer, toIs, valueResult.value),
new Decl.DefaultParam(identifer, toIs, valueResult.value, typeHint?.literal),
);
errors.push(valueResult.errors);
} while (this.matchToken(TokenType.comma));
Expand All @@ -343,13 +355,14 @@ export class Parser {
}

// parse lock statement
private declareLock(scope?: Decl.Scope): INodeResult<Decl.Lock> {
private declareLock(scope?: Decl.Scope, typeHint?: Maybe<ITypeHint>): INodeResult<Decl.Lock> {
const builder: NodeDataBuilder<Decl.Lock> = {
scope,
lock: undefined,
identifier: undefined,
value: undefined,
to: undefined,
typeHint,
};

builder.lock = this.previous();
Expand All @@ -366,18 +379,21 @@ export class Parser {
);
const valueResult = this.expression();
builder.value = valueResult.value;

builder.typeHint = this.types.find(x => x.ref === builder.identifier)?.literal;

this.terminal(Decl.Lock, builder);
return nodeResult(new Decl.Lock(builder), valueResult.errors);
}

// parse a variable declaration, scoping occurs elsewhere
private declareVariable(scope: Decl.Scope): INodeResult<Decl.Var> {
private declareVariable(scope: Decl.Scope, typeHint?: Maybe<ITypeHint>): INodeResult<Decl.Var> {
const builder: NodeDataBuilder<Decl.Var> = {
scope,
identifier: undefined,
value: undefined,
toIs: undefined,
typeHint,
};

builder.identifier = this.consumeIdentifierThrow(
Expand All @@ -395,6 +411,8 @@ export class Parser {
const valueResult = this.expression();
builder.value = valueResult.value;

builder.typeHint = this.types.find(x => x.ref === builder.identifier)?.literal;

this.terminal(Decl.Var, builder);
return nodeResult(new Decl.Var(builder), valueResult.errors);
}
Expand Down
10 changes: 8 additions & 2 deletions server/src/parser/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,16 @@ export interface IDeclScope extends RangeSequence {
toString(): string;
}

export interface ITypeHint extends RangeSequence {
typeHint: string;
toString(): string;
}

export type NodeDataBuilder<T> = Partial<Writeable<NodeData<T>>>;

export type NodeData<T> = Properties<
T,
Token | NodeBase | NodeBase[] | undefined
Token | NodeBase | NodeBase[] | ITypeHint | undefined
>;

export type SuffixTermTrailer =
Expand Down Expand Up @@ -191,7 +196,8 @@ export type TreeNode =
| IExpr
| ISuffixTerm
| IParameter
| IDeclScope;
| IDeclScope
| ITypeHint;

export interface IExprVisitable {
accept<T extends (...args: any) => any>(
Expand Down
Loading