Skip to content

Commit

Permalink
chore(compiler): store byte offsets in wing spans (#6094)
Browse files Browse the repository at this point in the history
This is a minor improvement that prevents the need to calculate line/column numbers when we're emitting errors for CLI error messages.

## Checklist

- [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [ ] Description explains motivation and solution
- [ ] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
Chriscbr authored Apr 1, 2024
1 parent d096652 commit 99016c4
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 80 deletions.
34 changes: 4 additions & 30 deletions apps/wing-console/console/server/src/utils/format-wing-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,6 @@ import { prettyPrintError } from "@winglang/sdk/lib/util/enhanced-error";
import type { File, Label } from "codespan-wasm";
import { CHARS_ASCII, emitDiagnostic } from "codespan-wasm";

function offsetFromLineAndColumn(source: string, line: number, column: number) {
const lines = source.split("\n");
let offset = 0;
for (let index = 0; index < line; index++) {
offset += lines[index]!.length + 1;
}
offset += column;
return offset;
}

export const formatWingError = async (error: unknown, entryPoint?: string) => {
try {
if (error instanceof CompileError) {
Expand All @@ -33,16 +23,8 @@ export const formatWingError = async (error: unknown, entryPoint?: string) => {
if (span !== null && span !== undefined && span.file_id) {
// `span` should only be null if source file couldn't be read etc.
const source = await readFile(span.file_id, "utf8");
const start = offsetFromLineAndColumn(
source,
span.start.line,
span.start.col,
);
const end = offsetFromLineAndColumn(
source,
span.end.line,
span.end.col,
);
const start = span.start_offset;
const end = span.end_offset;
const filePath = relative(cwd, span.file_id);
files.push({ name: filePath, source });
labels.push({
Expand All @@ -60,16 +42,8 @@ export const formatWingError = async (error: unknown, entryPoint?: string) => {
continue;
}
const source = await readFile(annotation.span.file_id, "utf8");
const start = offsetFromLineAndColumn(
source,
annotation.span.start.line,
annotation.span.start.col,
);
const end = offsetFromLineAndColumn(
source,
annotation.span.end.line,
annotation.span.end.col,
);
const start = annotation.span.start_offset;
const end = annotation.span.end_offset;
const filePath = relative(cwd, annotation.span.file_id);
files.push({ name: filePath, source });
labels.push({
Expand Down
29 changes: 4 additions & 25 deletions apps/wing/src/commands/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export async function compile(entrypoint?: string, options?: CompileOptions): Pr
if (span?.file_id) {
// `span` should only be null if source file couldn't be read etc.
const source = await fsPromise.readFile(span.file_id, "utf8");
const start = byteOffsetFromLineAndColumn(source, span.start.line, span.start.col);
const end = byteOffsetFromLineAndColumn(source, span.end.line, span.end.col);
const start = span.start_offset;
const end = span.end_offset;
const filePath = relative(cwd, span.file_id);
files.push({ name: filePath, source });
labels.push({
Expand All @@ -122,16 +122,8 @@ export async function compile(entrypoint?: string, options?: CompileOptions): Pr
continue;
}
const source = await fsPromise.readFile(annotation.span.file_id, "utf8");
const start = byteOffsetFromLineAndColumn(
source,
annotation.span.start.line,
annotation.span.start.col
);
const end = byteOffsetFromLineAndColumn(
source,
annotation.span.end.line,
annotation.span.end.col
);
const start = annotation.span.start_offset;
const end = annotation.span.end_offset;
const filePath = relative(cwd, annotation.span.file_id);
files.push({ name: filePath, source });
labels.push({
Expand Down Expand Up @@ -179,16 +171,3 @@ export async function compile(entrypoint?: string, options?: CompileOptions): Pr
}
}
}

function byteOffsetFromLineAndColumn(source: string, line: number, column: number) {
const lines = source.split("\n");
let offset = 0;
for (let i = 0; i < line; i++) {
offset += lines[i].length + 1;
}

// Convert char offset to byte offset
const encoder = new TextEncoder();
const srouce_bytes = encoder.encode(source.substring(0, offset));
return srouce_bytes.length + column;
}
33 changes: 33 additions & 0 deletions libs/wingc/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ pub struct WingSpan {
pub end: WingLocation,
/// Relative path to the file based on the entrypoint file
pub file_id: String,
/// Byte-level offsets into the file.
/// Not used for comparisons (start/end are used instead)
pub start_offset: usize,
pub end_offset: usize,
}

impl WingSpan {
Expand Down Expand Up @@ -213,10 +217,23 @@ impl WingSpan {
other.start
};
let end = if self.end > other.end { self.end } else { other.end };
let start_offset = if self.start_offset < other.start_offset {
self.start_offset
} else {
other.start_offset
};
let end_offset = if self.end_offset > other.end_offset {
self.end_offset
} else {
other.end_offset
};

Self {
start,
end,
file_id: self.file_id.clone(),
start_offset,
end_offset,
}
}

Expand Down Expand Up @@ -433,6 +450,8 @@ mod tests {
start: WingLocation { line: 0, col: 0 },
end: WingLocation { line: 1, col: 10 },
file_id: "test".to_string(),
start_offset: 0,
end_offset: 10,
};

let in_position = Position { line: 0, character: 5 };
Expand All @@ -450,17 +469,23 @@ mod tests {
start: WingLocation { line: 0, col: 0 },
end: WingLocation { line: 1, col: 10 },
file_id: file_id.to_string(),
start_offset: 0,
end_offset: 10,
};

let in_span = WingSpan {
start: WingLocation { line: 0, col: 5 },
end: WingLocation { line: 1, col: 5 },
file_id: file_id.to_string(),
start_offset: 0,
end_offset: 10,
};
let out_span = WingSpan {
start: WingLocation { line: 2, col: 0 },
end: WingLocation { line: 2, col: 5 },
file_id: file_id.to_string(),
start_offset: 0,
end_offset: 10,
};

assert!(span.contains_span(&in_span));
Expand All @@ -473,6 +498,8 @@ mod tests {
start: WingLocation { line: 0, col: 0 },
end: WingLocation { line: 1, col: 10 },
file_id: "test".to_string(),
start_offset: 0,
end_offset: 10,
};

let in_location = WingLocation { line: 0, col: 5 };
Expand All @@ -488,6 +515,8 @@ mod tests {
start: WingLocation { line: 1, col: 5 },
end: WingLocation { line: 1, col: 10 },
file_id: "test".to_string(),
start_offset: 0,
end_offset: 10,
};

let like_span1 = span1.clone();
Expand All @@ -496,11 +525,15 @@ mod tests {
start: WingLocation { line: 0, col: 0 },
end: WingLocation { line: 1, col: 1 },
file_id: "test".to_string(),
start_offset: 0,
end_offset: 10,
};
let later = WingSpan {
start: WingLocation { line: 2, col: 0 },
end: WingLocation { line: 2, col: 5 },
file_id: "test".to_string(),
start_offset: 0,
end_offset: 10,
};

assert!(span1 == like_span1);
Expand Down
4 changes: 4 additions & 0 deletions libs/wingc/src/jsify/codemaker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,8 @@ mod tests {
file_id: "test.w".to_string(),
start: WingLocation { line: 2, col: 4 },
end: WingLocation { line: 2, col: 8 },
start_offset: 0,
end_offset: 0,
};
let mut code = CodeMaker::with_source(&base_span);
code.line("1");
Expand Down Expand Up @@ -415,6 +417,8 @@ mod tests {
file_id: "test.w".to_string(),
start: WingLocation { line: 2, col: 4 },
end: WingLocation { line: 2, col: 8 },
start_offset: 0,
end_offset: 0,
};
let mut code = CodeMaker::with_source(&base_span);
code.line("1");
Expand Down
4 changes: 4 additions & 0 deletions libs/wingc/src/lsp/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,15 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse
start: parent.start_position().into(),
end: parent.end_position().into(),
file_id: file.to_string(),
start_offset: parent.start_byte(),
end_offset: parent.end_byte(),
}),
WingSpan {
start: node_to_complete.start_position().into(),
end: node_to_complete.end_position().into(),
file_id: file.to_string(),
start_offset: node_to_complete.start_byte(),
end_offset: node_to_complete.end_byte(),
},
params.text_document_position.position.into(),
&root_scope,
Expand Down
2 changes: 2 additions & 0 deletions libs/wingc/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,8 @@ impl<'s> Parser<'s> {
start: node_range.start_point.into(),
end: node_range.end_point.into(),
file_id: self.source_name.to_string(),
start_offset: node_range.start_byte,
end_offset: node_range.end_byte,
}
}

Expand Down
10 changes: 9 additions & 1 deletion libs/wingc/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5473,11 +5473,19 @@ impl<'a> TypeChecker<'a> {
let start = root.span.start;
let end = path.last().map(|s| s.span.end).unwrap_or(root.span.end);
let file_id = root.span.file_id.clone();
let start_offset = root.span.start_offset;
let end_offset = path.last().map(|s| s.span.end_offset).unwrap_or(root.span.end_offset);

Some(UserDefinedType {
root,
fields: path,
span: WingSpan { start, end, file_id },
span: WingSpan {
start,
end,
file_id,
start_offset,
end_offset,
},
})
}

Expand Down
42 changes: 18 additions & 24 deletions libs/wingcompiler/src/wingc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export async function load(options: WingCompilerLoadOptions) {
wasi_snapshot_preview1: wasi.wasiImport,
env: {
// This function is used only by the lsp
send_diagnostic: () => { },
send_diagnostic: () => {},
},
...(options.imports ?? {}),
} as any;
Expand Down Expand Up @@ -222,34 +222,28 @@ const HIGH_MASK = BigInt(32);
// From diagnostic.rs
export interface WingDiagnostic {
message: string;
span?: {
start: {
line: number;
col: number;
};
end: {
line: number;
col: number;
};
file_id: string;
};
span?: WingSpan;
annotations: {
message: string;
span: {
start: {
line: number;
col: number;
};
end: {
line: number;
col: number;
};
file_id: string;
};
span: WingSpan;
}[];
hints: string[];
}

export interface WingSpan {
start: {
line: number;
col: number;
};
end: {
line: number;
col: number;
};
file_id: string;
start_offset: number;
end_offset: number;
}

/**
* Runs the given WASM function in the Wing Compiler WASM instance.
*
Expand Down Expand Up @@ -282,7 +276,7 @@ export function invoke(
argMemoryBuffer.set(bytes);

const result = exports[func](argPointer, bytes.byteLength);

if (result === 0 || result === undefined || result === 0n) {
return 0;
} else {
Expand Down

0 comments on commit 99016c4

Please sign in to comment.