From f5af65a7011596e8b61dd9ee59aeda91897bb594 Mon Sep 17 00:00:00 2001 From: Noah Santschi-Cooney Date: Fri, 3 Aug 2018 15:46:17 +0100 Subject: [PATCH] Up-propogation of errors works. Now need to handle the case of changing includes --- server/src/graph.ts | 14 ++++-- server/src/linter.ts | 108 ++++++++++++++++++++++++------------------- server/src/server.ts | 4 +- tslint.json | 5 +- 4 files changed, 75 insertions(+), 56 deletions(-) diff --git a/server/src/graph.ts b/server/src/graph.ts index 5745fb9..d741254 100644 --- a/server/src/graph.ts +++ b/server/src/graph.ts @@ -1,5 +1,5 @@ type Node = { - parents: Map + parents: Map> children: Map } @@ -10,14 +10,14 @@ export class Graph { return this.nodes.has(uri) ? this.nodes.get(uri).parents.size > 0 : false } - public setParent(uri: string, parent: string) { + public setParent(uri: string, parent: string, lineNum: number) { const par: Node = this.nodes.has(parent) ? this.nodes.get(parent) : {parents: new Map(), children: new Map()} if (this.nodes.has(uri)) { const node = this.nodes.get(uri) - node.parents.set(parent, par) + node.parents.set(parent, {first: lineNum, second: par}) par.children.set(uri, node) } else { - const node: Node = {parents: new Map([par].map(p => [parent, p]) as [string, Node][]), children: new Map()} + const node: Node = {parents: new Map([par].map(p => [parent, {first: lineNum, second: p}]) as [string, Pair][]), children: new Map()} par.children.set(uri, node) this.nodes.set(uri, node) } @@ -28,4 +28,10 @@ export class Graph { if (!this.nodes.has(uri)) this.nodes.set(uri, {parents: new Map(), children: new Map()}) return this.nodes.get(uri) } +} + +// can you imagine that some people out there would import a whole library just for this? +export type Pair = { + first: T, + second: S } \ No newline at end of file diff --git a/server/src/linter.ts b/server/src/linter.ts index 60f8d50..4f421f4 100644 --- a/server/src/linter.ts +++ b/server/src/linter.ts @@ -98,7 +98,7 @@ export function preprocess(lines: string[], docURI: string) { lint(docURI, lines, includeMap, diagnostics) } -const buildIncludeGraph = (inc: IncludeObj) => includeGraph.setParent(inc.path, inc.parent) +const buildIncludeGraph = (inc: IncludeObj) => includeGraph.setParent(inc.path, inc.parent, inc.lineNum) function processIncludes(lines: string[], incStack: string[], allIncludes: IncludeObj[], diagnostics: Map, hasDirective: boolean) { const includes = getIncludes(incStack[0], lines) @@ -113,49 +113,61 @@ function processIncludes(lines: string[], incStack: string[], allIncludes: Inclu } } -// TODO no +type LinesProcessingInfo = { + total: number, + comment: Comment.State, + parStack: string[], + count: number[], +} + +// TODO can surely be reworked export function getIncludes(uri: string, lines: string[]) { - const count = [-1] // for each file we need to track the line number - const parStack = [uri] // for each include we need to track its parent + // the numbers start at -1 because we increment them as soon as we enter the loop so that we + // dont have to put an incrememnt at each return + const lineInfo: LinesProcessingInfo = { + total: -1, + comment: Comment.State.No, + parStack: [uri], + count: [-1], + } - let total = -1 // current line number overall - let comment = Comment.State.No + return lines.reduce((out, line, i, l): IncludeObj[] => processLine(out, line, lines, i, lineInfo), []) +} - return lines.reduce((out, line, i, l): IncludeObj[] => { - const updated = Comment.update(line, comment) - comment = updated[0] - line = updated[1] - lines[i] = line +function processLine(includes: IncludeObj[], line: string, lines: string[], i: number, linesInfo: LinesProcessingInfo): IncludeObj[] { + const updated = Comment.update(line, linesInfo.comment) + linesInfo.comment = updated[0] + line = updated[1] + lines[i] = line - count[count.length - 1]++ - total++ - if (comment) return out - if (line.startsWith('#line')) { - const inc = line.slice(line.indexOf('"') + 1, line.lastIndexOf('"')) + linesInfo.count[linesInfo.count.length - 1]++ + linesInfo.total++ + if (linesInfo.comment) return includes + if (line.startsWith('#line')) { + const inc = line.slice(line.indexOf('"') + 1, line.lastIndexOf('"')) - if (inc === parStack[parStack.length - 2]) { - count.pop() - parStack.pop() - } else { - count.push(-1) - parStack.push(inc) - } - return out + if (inc === linesInfo.parStack[linesInfo.parStack.length - 2]) { + linesInfo.count.pop() + linesInfo.parStack.pop() + } else { + linesInfo.count.push(-1) + linesInfo.parStack.push(inc) } + return includes + } - const match = line.match(reInclude) + const match = line.match(reInclude) - if (match) { - out.push({ - path: formatURI(absPath(parStack[parStack.length - 1], match[1])), - lineNum: count[count.length - 1], - lineNumTopLevel: total, - parent: formatURI(parStack[parStack.length - 1]), - match - }) - } - return out - }, []) + if (match) { + includes.push({ + path: formatURI(absPath(linesInfo.parStack[linesInfo.parStack.length - 1], match[1])), + lineNum: linesInfo.count[linesInfo.count.length - 1], + lineNumTopLevel: linesInfo.total, + parent: formatURI(linesInfo.parStack[linesInfo.parStack.length - 1]), + match + }) + } + return includes } function ifInvalidFile(inc: IncludeObj, lines: string[], incStack: string[], diagnostics: Map) { @@ -202,11 +214,9 @@ function mergeInclude(inc: IncludeObj, lines: string[], incStack: string[], diag incStack.push(inc.path) - // TODO deal with the fact that includes may not be the sole text on a line // add #line indicating we are entering a new include block lines[inc.lineNumTopLevel] = `#line 0 "${formatURI(inc.path)}"` // merge the lines of the file into the current document - // TODO do we wanna use a DLL here? lines.splice(inc.lineNumTopLevel + 1, 0, ...dataLines) // add the closing #line indicating we're re-entering a block a level up lines.splice(inc.lineNumTopLevel + 1 + dataLines.length, 0, `#line ${inc.lineNum + 1} "${inc.parent}"`) @@ -248,21 +258,23 @@ function lint(docURI: string, lines: string[], includes: Map }) } -function propogateDiagnostic(errorFile: string, type: string, line: string, msg: string, diagnostics: Map, includes: Map) { - let nextFile = errorFile - while (nextFile !== '0') { - // TODO this doesnt deal with the fact that an include can have multiple parents :( - const diag = { +function propogateDiagnostic(errorFile: string, type: string, line: string, msg: string, diagnostics: Map, includes: Map, parentURI?: string) { + includeGraph.get(parentURI || errorFile).parents.forEach((pair, parURI) => { + console.log('parent', parURI, 'child', errorFile) + const diag: Diagnostic = { severity: errorType(type), - range: calcRange(includes.get(nextFile).lineNum, includes.get(nextFile).parent), - message: `Line ${line} ${includes.get(errorFile).path.replace(conf.shaderpacksPath, '')} ${replaceWords(msg)}`, + range: calcRange(pair.first, parURI), + message: `Line ${line} ${errorFile.replace(conf.shaderpacksPath, '')} ${replaceWords(msg)}`, source: 'mc-glsl' } - diagnostics.get(includes.get(nextFile).parent).push(diag) + if (!diagnostics.has(parURI)) diagnostics.set(parURI, []) + diagnostics.get(parURI).push(diag) - nextFile = includes.get(nextFile).parent - } + if (pair.second.parents.size > 0) { + propogateDiagnostic(errorFile, type, line, msg, diagnostics, includes, parURI) + } + }) } export const replaceWords = (msg: string) => Array.from(tokens.entries()).reduce((acc, [key, value]) => acc.replace(key, value), msg) diff --git a/server/src/server.ts b/server/src/server.ts index 87298e4..2defb1e 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -55,9 +55,7 @@ export function onEvent(document: vsclangproto.TextDocument) { function lintBubbleDown(uri: string, document: vsclangproto.TextDocument) { includeGraph.get(uri).parents.forEach((parent, parentURI) => { - console.log(parentURI) - console.log(parent.parents) - if (parent.parents.size > 0) { + if (parent.second.parents.size > 0) { lintBubbleDown(parentURI, document) } else { const lines = getDocumentContents(parentURI).split('\n') diff --git a/tslint.json b/tslint.json index 06c86da..2cd1fd5 100644 --- a/tslint.json +++ b/tslint.json @@ -33,6 +33,9 @@ "severity": "warning" }, "curly": [true, "ignore-same-line"], - "whitespace": false + "whitespace": false, + "no-trailing-whitespace": { + "severity": "warning" + } } } \ No newline at end of file