Better diagnostics handling

Author: chenglou

CONTRIBUTING.md

@@ -11,11 +11,10 @@ They should be synced in from the `bsb` build. Don't take them from other places
 The build output is streamed into `lib/bs/.compiler.log`. Here are its various states, numbered here:
 
 1. Doesn't exist: artifacts not built yet, or cleaned away.
-2. Present but empty: currently building, no error yet.
-3. Present, non-empty, without a final line `# Done`: still building.
-4. Present, with the final line `# Done`: finished building.
+2. Present, without a final line `#Done`: still building.
+3. Present, with the final line `#Done`: finished building.
 
-Barring FS errors, there should be no other state to `.compiler.log`.
+Barring FS errors, there should be no other state to `.compiler.log`. Among others, this means the file is never present but empty.
 
 ### State 1
 
@@ -25,9 +24,8 @@ Artifacts cleaning through `bsb -clean` removes `.compiler.log` and turns into s
 
 After saving a file and running the build, the results stream into the log file. Unfortunately, UX-wise, in the editor, this might look like the diagnostics are suddenly gone then coming back in file by file. This looks bad. To remediate:
 
-- If the log file is in state 2 (see state numbers above), don't wipe the existing diagnostics yet.
-- If it's in state 3, update those particular files' diagnostics.
-- If in state 4, finish by clean up the rest of the old diagnostics. This means there's a bit of bookeeping needed here. Make sure you get it right. It's possible for a build to be interrupted (and therefore state 4 never reached) and restarted.
+- If it's in state 2, update those particular files' diagnostics but don't wipe the files' diagnostics yet.
+- If in state 3, finish by clean up the rest of the old diagnostics. This means there's a bit of bookeeping needed here. Make sure you get it right. It's possible for a build to be interrupted (and therefore state 4 never reached) and restarted.
 
 Even this fix isn't great. Ideally, the editor's diagnostics can be greyed out while we're updating them...
 

server/src/server.ts

@@ -23,16 +23,14 @@ let previouslyDiagnosedFiles: Set<string> = new Set()
 let compilerLogPaths: Set<string> = new Set()
 
 let sendUpdatedDiagnostics = () => {
-  let diagnosedFiles: { [key: string]: t.Diagnostic[] } = {}
+  let diagnosedFiles: utils.filesDiagnostics = {}
   compilerLogPaths.forEach(compilerLogPath => {
     let content = fs.readFileSync(compilerLogPath, { encoding: 'utf-8' });
     console.log("new log content: ", content)
-    let filesAndErrors = utils.parseCompilerLogOutput(content, ":")
-    Object.keys(filesAndErrors).forEach(file => {
-      // assumption: there's no existing files[file] entry
-      // this is true; see the lines above. A file can only belong to one .compiler.log root
-      diagnosedFiles[file] = filesAndErrors[file]
-    })
+    let { done: buildDone, result: filesAndErrors } = utils.parseCompilerLogOutput(content)
+    // A file can only belong to one .compiler.log root. So we're not overriding
+    // any existing key here
+    Object.assign(diagnosedFiles, filesAndErrors)
   });
 
   // Send new diagnostic, wipe old ones

server/src/utils.ts

@@ -88,9 +88,18 @@ export let parseDiagnosticLocation = (location: string): Range => {
 	}
 }
 
-export let parseCompilerLogOutput = (content: string, separator: string) => {
+export type filesDiagnostics = {
+	[key: string]: p.Diagnostic[];
+}
+type parsedCompilerLogResult = {
+	done: boolean,
+	result: filesDiagnostics,
+}
+export let parseCompilerLogOutput = (content: string): parsedCompilerLogResult => {
 	/* example .compiler.log file content that we're gonna parse:
 
+#Start(1600519680823)
+
 	Syntax error!
 	/Users/chenglou/github/reason-react/src/test.res:1:8-2:3
 
@@ -121,11 +130,10 @@ export let parseCompilerLogOutput = (content: string, separator: string) => {
 	2 │ let b = "hi"
 	3 │ let a = b + 1
 
-	This has type:
-		string
+	This has type: string
+	Somewhere wanted: int
 
-	But somewhere wanted:
-		int
+#Done(1600519680836)
 	*/
 
 	type parsedDiagnostic = {
@@ -136,6 +144,8 @@ export let parseCompilerLogOutput = (content: string, separator: string) => {
 	}
 	let parsedDiagnostics: parsedDiagnostic[] = [];
 	let lines = content.split('\n');
+	let done = false;
+
 	for (let i = 0; i < lines.length; i++) {
 		let line = lines[i];
 		if (line.startsWith('  We\'ve found a bug for you!')) {
@@ -184,22 +194,23 @@ export let parseCompilerLogOutput = (content: string, separator: string) => {
 				tag: undefined,
 				content: []
 			})
+		} else if (line.startsWith('#Done(')) {
+			done = true
 		} else if (/^  +[0-9]+ /.test(line)) {
 			// code display. Swallow
 		} else if (line.startsWith('  ')) {
 			parsedDiagnostics[parsedDiagnostics.length - 1].content.push(line)
 		}
 	}
 
-	// map of file path to list of diagnostic
-	let ret: { [key: string]: t.Diagnostic[] } = {}
+	let result: filesDiagnostics = {}
 	parsedDiagnostics.forEach(parsedDiagnostic => {
 		let [fileAndLocation, ...diagnosticMessage] = parsedDiagnostic.content
-		let locationSeparator = fileAndLocation.indexOf(separator)
+		let locationSeparator = fileAndLocation.indexOf(':')
 		let file = fileAndLocation.substring(2, locationSeparator)
 		let location = fileAndLocation.substring(locationSeparator + 1)
-		if (ret[file] == null) {
-			ret[file] = []
+		if (result[file] == null) {
+			result[file] = []
 		}
 		let cleanedUpDiagnostic = diagnosticMessage
 			.map(line => {
@@ -209,7 +220,7 @@ export let parseCompilerLogOutput = (content: string, separator: string) => {
 			.join('\n')
 			// remove start and end whitespaces/newlines
 			.trim() + '\n';
-		ret[file].push({
+		result[file].push({
 			severity: parsedDiagnostic.severity,
 			tags: parsedDiagnostic.tag === undefined ? [] : [parsedDiagnostic.tag],
 			code: parsedDiagnostic.code,
@@ -219,5 +230,5 @@ export let parseCompilerLogOutput = (content: string, separator: string) => {
 		})
 	})
 
-	return ret
+	return { done, result }
 }