From 682e677ee9628c3efb1b8dc71aaeb0da3a7dd4e2 Mon Sep 17 00:00:00 2001 From: Noah Santschi-Cooney Date: Wed, 18 Jul 2018 18:52:43 +0100 Subject: [PATCH] Errors in includes now propogate up the include tree --- server/src/linter.ts | 146 ++++++++++++++++++++++++------------------- server/src/server.ts | 1 + 2 files changed, 83 insertions(+), 64 deletions(-) diff --git a/server/src/linter.ts b/server/src/linter.ts index 1ebefaa..70eb102 100644 --- a/server/src/linter.ts +++ b/server/src/linter.ts @@ -21,7 +21,8 @@ const files = new Map() type IncludeObj = { lineNum: number, - lineNumParent: number, + lineNumTopLevel: number, + path: string, parent: string, match: RegExpMatchArray } @@ -65,6 +66,14 @@ enum Comment { Multi } +export function isInComment(line: string, state: Comment): Comment { + const indexOf = line.indexOf('#include') + if (indexOf > -1 && line.indexOf('//') < indexOf) { + return Comment.No + } + return Comment.No +} + export function preprocess(lines: string[], docURI: string) { if (lines.find((value: string, i: number, obj: string[]): boolean => reIncludeExt.test(value)) == undefined) { for (let i = 0; i < lines.length; i++) { @@ -81,68 +90,41 @@ export function preprocess(lines: string[], docURI: string) { } const incStack = [docURI] - processIncludes(lines, incStack) + const allIncludes: IncludeObj[] = [] + processIncludes(lines, incStack, allIncludes) try { - lint(docURI, lines, incStack) + lint(docURI, lines, new Map(allIncludes.map(obj => [obj.path, obj]) as [string, IncludeObj][])) } catch (e) { console.log(e) } } -function processIncludes(lines: string[], incStack: string[]) { +function processIncludes(lines: string[], incStack: string[], allIncludes: IncludeObj[]) { const includes = getIncludes(incStack[0], lines) + allIncludes.push(...includes) if (includes.length > 0) { includes.reverse().forEach(inc => { mergeInclude(inc, lines, incStack) }) // recursively check for more includes to be merged - processIncludes(lines, incStack) + processIncludes(lines, incStack, allIncludes) } } -function mergeInclude(inc: IncludeObj, lines: string[], incStack: string[]) { - const incPath = absPath(inc.parent, inc.match[1]) - if (!incPath) return false - if (!existsSync(incPath)) { - const range = calcRange(inc.lineNumParent, incStack[0]) - // TODO this needs to be aggregated and passed to the lint function, else theyre overwritten - connection.sendDiagnostics({uri: 'file://' + inc.parent, diagnostics: [{ - severity: DiagnosticSeverity.Error, - range, - message: `${incPath} is missing.`, - source: 'mc-glsl' - }]}) - lines[inc.lineNumParent] = '' - return - } - const dataLines = readFileSync(incPath).toString().split('\n') - incStack.push(incPath) - - // 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.lineNumParent] = `#line 0 "${incPath}"` - // merge the lines of the file into the current document - lines.splice(inc.lineNumParent + 1, 0, ...dataLines) - // add the closing #line indicating we're re-entering a block a level up - lines.splice(inc.lineNumParent + 1 + dataLines.length, 0, `#line ${inc.lineNum} "${inc.parent}"`) -} - // TODO no export function getIncludes(uri: string, lines: string[]) { - const out: IncludeObj[] = [] - 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 let total = -1 // current line number overall let comment = Comment.No - lines.forEach(line => { + return lines.reduce((out: IncludeObj[], line: string, i, l): IncludeObj[] => { comment = isInComment(line, comment) count[count.length - 1]++ total++ - if (comment) return + if (comment) return out if (line.startsWith('#line')) { const inc = line.slice(line.indexOf('"') + 1, line.lastIndexOf('"')) @@ -153,48 +135,50 @@ export function getIncludes(uri: string, lines: string[]) { count.push(-1) parStack.push(inc) } - return + return out } const match = line.match(reInclude) if (match) { out.push({ + path: absPath(parStack[parStack.length - 1], match[1]), lineNum: count[count.length - 1], - lineNumParent: total, + lineNumTopLevel: total, parent: parStack[parStack.length - 1], match }) } - }) - return out + return out + }, []) } -export const formatURI = (uri: string) => uri.replace(/^file:\/\//, '') - -export function isInComment(line: string, state: Comment): Comment { - const indexOf = line.indexOf('#include') - if (indexOf > -1 && line.indexOf('//') < indexOf) { - return Comment.No - } - return Comment.No -} - -export function absPath(currFile: string, includeFile: string): string { - if (!currFile.startsWith(conf.shaderpacksPath) || conf.shaderpacksPath === '') { - connection.window.showErrorMessage(`Shaderpacks path may not be correct. Current file is in '${currFile}' but the path is set to '${conf.shaderpacksPath}'`) +function mergeInclude(inc: IncludeObj, lines: string[], incStack: string[]) { + if (!existsSync(inc.path)) { + const range = calcRange(inc.lineNumTopLevel, incStack[0]) + // TODO this needs to be aggregated and passed to the lint function, else theyre overwritten + connection.sendDiagnostics({uri: 'file://' + inc.parent, diagnostics: [{ + severity: DiagnosticSeverity.Error, + range, + message: `${inc.path} is missing.`, + source: 'mc-glsl' + }]}) + lines[inc.lineNumTopLevel] = '' return } + const dataLines = readFileSync(inc.path).toString().split('\n') + incStack.push(inc.path) - // TODO add explanation comment - if (includeFile.charAt(0) === '/') { - const shaderPath = currFile.replace(conf.shaderpacksPath, '').split('/').slice(0, 3).join('/') - return path.join(conf.shaderpacksPath, shaderPath, includeFile) - } - return path.join(path.dirname(currFile), includeFile) + // 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 "${inc.path}"` + // merge the lines of the file into the current document + 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} "${inc.parent}"`) } -function lint(uri: string, lines: string[], includes: string[]) { +function lint(uri: string, lines: string[], includes: Map) { console.log(lines.join('\n')) //return let out: string = '' @@ -205,19 +189,35 @@ function lint(uri: string, lines: string[], includes: string[]) { } const diagnostics = new Map([[uri, Array()]]) - includes.forEach(obj => diagnostics.set(obj, [])) + includes.forEach(obj => diagnostics.set(obj.path, [])) filterMatches(out).forEach((match) => { - const [whole, type, file, line, msg] = match + let [whole, type, file, line, msg] = match - const diag: Diagnostic = { - severity: type === 'ERROR' ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning, + let diag: Diagnostic = { + severity: errorType(type), range: calcRange(parseInt(line) - 1, file.length - 1 ? file : uri), message: replaceWord(msg), source: 'mc-glsl' } diagnostics.get(file.length - 1 ? file : uri).push(diag) + + // if is an include, highlight an error in the parents line of inclusion + while (file !== '0' && file !== uri) { + // TODO what if we dont know the top level parent? Feel like thats a non-issue given that we have uri + // TODO prefix error with filename + diag = { + severity: errorType(type), + range: calcRange(includes.get(file).lineNum, includes.get(file).parent), + message: replaceWord(msg), + source: 'mc-glsl' + } + + diagnostics.get(includes.get(file).parent).push(diag) + + file = includes.get(file).parent + } }) daigsArray(diagnostics).forEach(d => { @@ -227,6 +227,8 @@ function lint(uri: string, lines: string[], includes: string[]) { export const replaceWord = (msg: string) => Array.from(tokens.entries()).reduce((acc, [key, value]) => acc.replace(key, value), msg) +const errorType = (error: string) => error === 'ERROR' ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning + const daigsArray = (diags: Map) => Array.from(diags).map(kv => ({uri: 'file://' + kv[0], diag: kv[1]})) const filterMatches = (output: string) => output @@ -249,4 +251,20 @@ function calcRange(lineNum: number, uri: string): Range { const startOfLine = line.length - line.trimLeft().length const endOfLine = line.slice(0, line.indexOf('//')).trimRight().length + 1 return Range.create(lineNum, startOfLine, lineNum, endOfLine) +} + +export const formatURI = (uri: string) => uri.replace(/^file:\/\//, '') + +export function absPath(currFile: string, includeFile: string): string { + if (!currFile.startsWith(conf.shaderpacksPath) || conf.shaderpacksPath === '') { + connection.window.showErrorMessage(`Shaderpacks path may not be correct. Current file is in '${currFile}' but the path is set to '${conf.shaderpacksPath}'`) + return + } + + // TODO add explanation comment + if (includeFile.charAt(0) === '/') { + const shaderPath = currFile.replace(conf.shaderpacksPath, '').split('/').slice(0, 3).join('/') + return path.join(conf.shaderpacksPath, shaderPath, includeFile) + } + return path.join(path.dirname(currFile), includeFile) } \ No newline at end of file diff --git a/server/src/server.ts b/server/src/server.ts index 082cad0..4bbc303 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -44,6 +44,7 @@ export function onEvent(document: vsclangproto.TextDocument) { preprocess(document.getText().split('\n'), formatURI(document.uri)) } catch (e) { connection.window.showErrorMessage(`[mc-glsl] ${e.message}`) + throw(e) } }