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

feat: adding depth information to env-diff, remove env names for unused #817

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions src/dataflow/environments/append.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ function uniqueMergeValues(old: IdentifierDefinition[], value: readonly Identifi

function appendIEnvironmentWith(base: IEnvironment | undefined, next: IEnvironment | undefined): IEnvironment {
guard(base !== undefined && next !== undefined, 'can not append environments with undefined')
guard(base.name === next.name, 'cannot overwrite environments with different names')
const map = new Map(base.memory)
for(const [key, value] of next.memory) {
const old = map.get(key)
Expand All @@ -29,7 +28,7 @@ function appendIEnvironmentWith(base: IEnvironment | undefined, next: IEnvironme

const parent = base.parent === BuiltInEnvironment ? BuiltInEnvironment : appendIEnvironmentWith(base.parent, next.parent)

const out = new Environment(base.name, parent)
const out = new Environment(parent)
out.memory = map
return out
}
Expand Down
2 changes: 1 addition & 1 deletion src/dataflow/environments/clone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function cloneEnvironment(environment: IEnvironment | undefined, recurseParents:
} else if(environment.id === BuiltInEnvironment.id) {
return BuiltInEnvironment
}
const clone = new Environment(environment.name, recurseParents ? cloneEnvironment(environment.parent, recurseParents) : environment.parent)
const clone = new Environment(recurseParents ? cloneEnvironment(environment.parent, recurseParents) : environment.parent)
clone.memory = new Map(JSON.parse(JSON.stringify([...environment.memory])) as [Identifier, IdentifierDefinition[]][])
return clone
}
Expand Down
20 changes: 10 additions & 10 deletions src/dataflow/environments/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,25 +57,22 @@
}
}

export function diffEnvironment<Report extends WriteableDifferenceReport>(a: IEnvironment | undefined, b: IEnvironment | undefined, info: GenericDifferenceInformation<Report>): void {
export function diffEnvironment<Report extends WriteableDifferenceReport>(a: IEnvironment | undefined, b: IEnvironment | undefined, info: GenericDifferenceInformation<Report>, depth: number): void {
if(a === undefined || b === undefined) {
if(a !== b) {
info.report.addComment(`${info.position}Different environments. ${info.leftname}: ${a !== undefined ? 'present' : 'undefined'} vs. ${info.rightname}: ${b !== undefined ? 'present' : 'undefined'}`)
info.report.addComment(`${info.position}[at level: ${depth}] Different environments. ${info.leftname}: ${a !== undefined ? 'present' : 'undefined'} vs. ${info.rightname}: ${b !== undefined ? 'present' : 'undefined'}`)
}
return
}
if(a.name !== b.name) {
info.report.addComment(`${info.position}Different environment names. ${info.leftname}: ${a.name} vs. ${info.rightname}: ${b.name}`)
}
if(a.memory.size !== b.memory.size) {
info.report.addComment(`${info.position}Different environment sizes. ${info.leftname}: ${a.memory.size} vs. ${info.rightname}: ${b.memory.size}`)
info.report.addComment(`${info.position}[at level: ${depth}] Different number of definitions in environment. ${info.leftname}: ${a.memory.size} vs. ${info.rightname}: ${b.memory.size}`)

Check warning on line 68 in src/dataflow/environments/diff.ts

View check run for this annotation

Codecov / codecov/patch

src/dataflow/environments/diff.ts#L68

Added line #L68 was not covered by tests
setDifference(new Set([...a.memory.keys()]), new Set([...b.memory.keys()]), {
...info,
position: `${info.position}Key comparison. `
position: `${info.position}[at level: ${depth}] Key comparison. `
})
}
diffMemory(a, b, info)
diffEnvironment(a.parent, b.parent, { ...info, position: `${info.position}Parents of ${a.id} & ${b.id}. ` })
diffMemory(a, b, { ...info, position: `${info.position}[at level: ${depth}] ` })
diffEnvironment(a.parent, b.parent, { ...info, position: `${info.position}Parents of ${a.id} & ${b.id}. ` }, depth--)
}

export function diffEnvironmentInformation<Report extends WriteableDifferenceReport>(a: REnvironmentInformation | undefined, b: REnvironmentInformation | undefined, info: GenericDifferenceInformation<Report>): void {
Expand All @@ -85,5 +82,8 @@
}
return
}
diffEnvironment(a.current, b.current, info)
if(a.level !== b.level) {
info.report.addComment(`${info.position}Different environment levels: ${info.leftname}: ${a.level} vs. ${info.rightname}: ${b.level}. Using max to report level for further errors.`)

Check warning on line 86 in src/dataflow/environments/diff.ts

View check run for this annotation

Codecov / codecov/patch

src/dataflow/environments/diff.ts#L86

Added line #L86 was not covered by tests
}
diffEnvironment(a.current, b.current, info, Math.max(a.level, b.level))
}
23 changes: 9 additions & 14 deletions src/dataflow/environments/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,26 +45,23 @@ export function makeAllMaybe(references: readonly IdentifierReference[] | undefi

export interface IEnvironment {
/** unique and internally generated identifier -- will not be used for comparison but assists debugging for tracking identities */
readonly id: string
readonly name: string
readonly id: string
/** Lexical parent of the environment, if any (can be manipulated by R code) */
parent: IEnvironment
parent: IEnvironment
/**
* Maps to exactly one definition of an identifier if the source is known, otherwise to a list of all possible definitions
*/
memory: Map<Identifier, IdentifierDefinition[]>
memory: Map<Identifier, IdentifierDefinition[]>
}

let environmentIdCounter = 0

export class Environment implements IEnvironment {
readonly name: string
readonly id: string = `${environmentIdCounter++}`
parent: IEnvironment
memory: Map<Identifier, IdentifierDefinition[]>
readonly id: string = `${environmentIdCounter++}`
parent: IEnvironment
memory: Map<Identifier, IdentifierDefinition[]>

constructor(name: string, parent: IEnvironment) {
this.name = name
constructor(parent: IEnvironment) {
this.parent = parent
this.memory = new Map()
}
Expand All @@ -86,15 +83,13 @@ export interface REnvironmentInformation {


/* the built-in environment is the root of all environments */
export const BuiltInEnvironment = new Environment('built-in', undefined as unknown as IEnvironment)
export const BuiltInEnvironment = new Environment(undefined as unknown as IEnvironment)

BuiltInEnvironment.memory = BuiltInMemory

export const GLOBAL_ENV_NAME = 'global'

export function initializeCleanEnvironments(): REnvironmentInformation {
return {
current: new Environment(GLOBAL_ENV_NAME, BuiltInEnvironment),
current: new Environment(BuiltInEnvironment),
level: 0
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/dataflow/environments/overwrite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ function anyIsMaybeOrEmpty(values: readonly IdentifierDefinition[]): boolean {

export function overwriteIEnvironmentWith(base: IEnvironment | undefined, next: IEnvironment | undefined, includeParent = true): IEnvironment {
guard(base !== undefined && next !== undefined, 'can not overwrite environments with undefined')
guard(base.name === next.name, 'cannot overwrite environments with different names')
const map = new Map(base.memory)
for(const [key, values] of next.memory) {
const hasMaybe = anyIsMaybeOrEmpty(values)
Expand All @@ -44,7 +43,7 @@ export function overwriteIEnvironmentWith(base: IEnvironment | undefined, next:
parent = base.parent
}

const out = new Environment(base.name, parent)
const out = new Environment(parent)
out.memory = map
return out
}
Expand Down
2 changes: 1 addition & 1 deletion src/dataflow/environments/scoping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { guard } from '../../util/assert'
/** Add a new local environment scope to the stack, returns the modified variant - sharing the original environments in the stack (no deep-clone) */
export function pushLocalEnvironment(base: REnvironmentInformation): REnvironmentInformation {
return {
current: new Environment('local', base.current),
current: new Environment(base.current),
level: base.level + 1
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/util/mermaid/dfg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function printEnvironmentToLines(env: IEnvironment | undefined): string[] {
} else if(env.id === BuiltInEnvironment.id) {
return ['Built-in']
}
const lines = [...printEnvironmentToLines(env.parent), `${env.id}--${env.name}${'-'.repeat(40)}`]
const lines = [...printEnvironmentToLines(env.parent), `${env.id}${'-'.repeat(40)}`]
const longestName = Math.max(...[...env.memory.keys()].map(x => x.length))
for(const [name, defs] of env.memory.entries()) {
const printName = `${name}:`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ describe('Initialization', () => {
const clean = initializeCleanEnvironments()
expect(clean.current,'there should be a current environment').to.be.not.undefined
expect(clean.current.memory.size, 'the current environment should have no memory').to.be.equal(0)
expect(clean.current.name, 'the current environment must have the correct scope name').to.be.equal('global')
expect(clean.level, 'the level of the clean environment is predefined as 0').to.be.equal(0)
})
it(label('Clean creation should create independent new environments', ['lexicographic-scope'], ['other']), () => {
const clean = initializeCleanEnvironments()
clean.current.parent = new Environment('test', clean.current.parent)
clean.current.parent = new Environment(clean.current.parent)

const second = initializeCleanEnvironments()
expect(second.current.parent.id, 'the new one should have a parent, the built-in environment').to.be.equal(BuiltInEnvironment.id)
Expand Down
Loading