Skip to content

Commit

Permalink
respect package.json indentation in bun install (#12328)
Browse files Browse the repository at this point in the history
  • Loading branch information
dylan-conway authored Jul 4, 2024
1 parent 39d5c8a commit 4fefb85
Show file tree
Hide file tree
Showing 12 changed files with 297 additions and 82 deletions.
2 changes: 1 addition & 1 deletion src/bun.js/api/BunObject.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3880,7 +3880,7 @@ const TOMLObject = struct {
return .zero;
};
var writer = js_printer.BufferPrinter.init(buffer_writer);
_ = js_printer.printJSON(*js_printer.BufferPrinter, &writer, parse_result, &source) catch {
_ = js_printer.printJSON(*js_printer.BufferPrinter, &writer, parse_result, &source, .{}) catch {
globalThis.throwValue(log.toJS(globalThis, default_allocator, "Failed to print toml"));
return .zero;
};
Expand Down
1 change: 1 addition & 0 deletions src/bun.js/api/server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6166,6 +6166,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
&writer,
bun.Global.BunInfo.generate(*Bundler, &JSC.VirtualMachine.get().bundler, allocator) catch unreachable,
&source,
.{},
) catch unreachable;

resp.writeStatus("200 OK");
Expand Down
12 changes: 5 additions & 7 deletions src/bundler/bundle_v2.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6785,12 +6785,11 @@ const LinkerContext = struct {
const runtimeRequireRef = if (c.resolver.opts.target.isBun()) null else c.graph.symbols.follow(runtime_members.get("__require").?.ref);

{
const indent: usize = 0;
// TODO: IIFE indent

const print_options = js_printer.Options{
// TODO: IIFE
.indent = indent,
// TODO: IIFE indent
.indent = .{},
.has_run_symbol_renamer = true,

.allocator = worker.allocator,
Expand Down Expand Up @@ -7678,8 +7677,8 @@ const LinkerContext = struct {
}

const print_options = js_printer.Options{
// TODO: IIFE
.indent = 0,
// TODO: IIFE indent
.indent = .{},
.has_run_symbol_renamer = true,

.allocator = allocator,
Expand Down Expand Up @@ -8959,8 +8958,7 @@ const LinkerContext = struct {

const print_options = js_printer.Options{
// TODO: IIFE
.indent = 0,

.indent = .{},
.commonjs_named_exports = ast.commonjs_named_exports,
.commonjs_named_exports_ref = ast.exports_ref,
.commonjs_named_exports_deoptimized = flags.wrap == .cjs,
Expand Down
2 changes: 1 addition & 1 deletion src/cli/create_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1436,7 +1436,7 @@ pub const CreateCommand = struct {

const package_json_writer = JSPrinter.NewFileWriter(package_json_file.?);

const written = JSPrinter.printJSON(@TypeOf(package_json_writer), package_json_writer, package_json_expr, &source) catch |err| {
const written = JSPrinter.printJSON(@TypeOf(package_json_writer), package_json_writer, package_json_expr, &source, .{}) catch |err| {
Output.prettyErrorln("package.json failed to write due to error {s}", .{@errorName(err)});
package_json_file = null;
break :process_package_json;
Expand Down
1 change: 1 addition & 0 deletions src/cli/init_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ pub const InitCommand = struct {
package_json_writer,
js_ast.Expr{ .data = .{ .e_object = fields.object }, .loc = logger.Loc.Empty },
&logger.Source.initEmptyFile("package.json"),
.{},
) catch |err| {
Output.prettyErrorln("package.json failed to write due to error {s}", .{@errorName(err)});
package_json_file = null;
Expand Down
2 changes: 1 addition & 1 deletion src/cli/pm_trusted_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ pub const TrustCommand = struct {
buffer_writer.append_newline = package_json_contents.len > 0 and package_json_contents[package_json_contents.len - 1] == '\n';
var package_json_writer = bun.js_printer.BufferPrinter.init(buffer_writer);

_ = bun.js_printer.printJSON(@TypeOf(&package_json_writer), &package_json_writer, package_json, &package_json_source) catch |err| {
_ = bun.js_printer.printJSON(@TypeOf(&package_json_writer), &package_json_writer, package_json, &package_json_source, .{}) catch |err| {
Output.errGeneric("failed to print package.json: {s}", .{@errorName(err)});
Global.crash();
};
Expand Down
91 changes: 73 additions & 18 deletions src/install/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2718,13 +2718,15 @@ pub const PackageManager = struct {
pub const MapEntry = struct {
root: Expr,
source: logger.Source,
indentation: JSPrinter.Options.Indentation = .{},
};

pub const Map = bun.StringHashMapUnmanaged(MapEntry);

pub const GetJSONOptions = struct {
init_reset_store: bool = true,
always_decode_escape_sequences: bool = true,
guess_indentation: bool = false,
};

pub const GetResult = union(enum) {
Expand All @@ -2745,8 +2747,14 @@ pub const PackageManager = struct {
/// Given an absolute path to a workspace package.json, return the AST
/// and contents of the file. If the package.json is not present in the
/// cache, it will be read from disk and parsed, and stored in the cache.
pub fn getWithPath(this: *@This(), allocator: std.mem.Allocator, log: *logger.Log, abs_package_json_path: anytype, comptime opts: GetJSONOptions) GetResult {
bun.assert(std.fs.path.isAbsolute(abs_package_json_path));
pub fn getWithPath(
this: *@This(),
allocator: std.mem.Allocator,
log: *logger.Log,
abs_package_json_path: anytype,
comptime opts: GetJSONOptions,
) GetResult {
bun.assertWithLocation(std.fs.path.isAbsolute(abs_package_json_path), @src());

var buf: if (Environment.isWindows) bun.PathBuffer else void = undefined;
const path = if (comptime !Environment.isWindows)
Expand All @@ -2769,16 +2777,25 @@ pub const PackageManager = struct {
if (comptime opts.init_reset_store)
initializeStore();

const _json = if (comptime opts.always_decode_escape_sequences)
json_parser.ParsePackageJSONUTF8AlwaysDecode(&source, log, allocator)
else
json_parser.ParsePackageJSONUTF8(&source, log, allocator);
const json_result = json_parser.ParsePackageJSONUTF8WithOpts(
&source,
log,
allocator,
.{
.is_json = true,
.allow_comments = true,
.allow_trailing_commas = true,
.always_decode_escape_sequences = opts.always_decode_escape_sequences,
.guess_indentation = opts.guess_indentation,
},
);

const json = _json catch |err| return .{ .parse_err = err };
const json = json_result catch |err| return .{ .parse_err = err };

entry.value_ptr.* = .{
.root = json.deepClone(allocator) catch bun.outOfMemory(),
.root = json.root.deepClone(allocator) catch bun.outOfMemory(),
.source = source,
.indentation = json.indentation,
};

entry.key_ptr.* = key;
Expand All @@ -2794,7 +2811,7 @@ pub const PackageManager = struct {
source: logger.Source,
comptime opts: GetJSONOptions,
) GetResult {
bun.assert(std.fs.path.isAbsolute(source.path.text));
bun.assertWithLocation(std.fs.path.isAbsolute(source.path.text), @src());

var buf: if (Environment.isWindows) bun.PathBuffer else void = undefined;
const path = if (comptime !Environment.isWindows)
Expand All @@ -2813,15 +2830,25 @@ pub const PackageManager = struct {
if (comptime opts.init_reset_store)
initializeStore();

const _json = if (comptime opts.always_decode_escape_sequences)
json_parser.ParsePackageJSONUTF8AlwaysDecode(&source, log, allocator)
else
json_parser.ParsePackageJSONUTF8(&source, log, allocator);
const json = _json catch |err| return .{ .parse_err = err };
const json_result = json_parser.ParsePackageJSONUTF8WithOpts(
&source,
log,
allocator,
.{
.is_json = true,
.allow_comments = true,
.allow_trailing_commas = true,
.always_decode_escape_sequences = opts.always_decode_escape_sequences,
.guess_indentation = opts.guess_indentation,
},
);

const json = json_result catch |err| return .{ .parse_err = err };

entry.value_ptr.* = .{
.root = json.deepClone(allocator) catch bun.outOfMemory(),
.root = json.root.deepClone(allocator) catch bun.outOfMemory(),
.source = source,
.indentation = json.indentation,
};

entry.key_ptr.* = allocator.dupe(u8, path) catch bun.outOfMemory();
Expand Down Expand Up @@ -9791,6 +9818,7 @@ pub const PackageManager = struct {
manager.original_package_json_path,
.{
.always_decode_escape_sequences = false,
.guess_indentation = true,
},
)) {
.parse_err => |err| {
Expand All @@ -9814,6 +9842,7 @@ pub const PackageManager = struct {
},
.entry => |entry| entry,
};
const current_package_json_indent = current_package_json.indentation;

// If there originally was a newline at the end of their package.json, preserve it
// so that we don't cause unnecessary diffs in their git history.
Expand Down Expand Up @@ -9950,7 +9979,15 @@ pub const PackageManager = struct {
buffer_writer.append_newline = preserve_trailing_newline_at_eof_for_package_json;
var package_json_writer = JSPrinter.BufferPrinter.init(buffer_writer);

var written = JSPrinter.printJSON(@TypeOf(&package_json_writer), &package_json_writer, current_package_json.root, &current_package_json.source) catch |err| {
var written = JSPrinter.printJSON(
@TypeOf(&package_json_writer),
&package_json_writer,
current_package_json.root,
&current_package_json.source,
.{
.indent = current_package_json_indent,
},
) catch |err| {
Output.prettyErrorln("package.json failed to write due to error {s}", .{@errorName(err)});
Global.crash();
};
Expand Down Expand Up @@ -9980,7 +10017,14 @@ pub const PackageManager = struct {

// The lifetime of this pointer is only valid until the next call to `getWithPath`, which can happen after this scope.
// https://github.com/oven-sh/bun/issues/12288
const root_package_json = switch (manager.workspace_package_json_cache.getWithPath(manager.allocator, manager.log, root_package_json_path, .{})) {
const root_package_json = switch (manager.workspace_package_json_cache.getWithPath(
manager.allocator,
manager.log,
root_package_json_path,
.{
.guess_indentation = true,
},
)) {
.parse_err => |err| {
switch (Output.enable_ansi_colors) {
inline else => |enable_ansi_colors| {
Expand Down Expand Up @@ -10015,7 +10059,15 @@ pub const PackageManager = struct {
buffer_writer2.append_newline = preserve_trailing_newline_at_eof_for_package_json;
var package_json_writer2 = JSPrinter.BufferPrinter.init(buffer_writer2);

_ = JSPrinter.printJSON(@TypeOf(&package_json_writer2), &package_json_writer2, root_package_json.root, &root_package_json.source) catch |err| {
_ = JSPrinter.printJSON(
@TypeOf(&package_json_writer2),
&package_json_writer2,
root_package_json.root,
&root_package_json.source,
.{
.indent = root_package_json.indentation,
},
) catch |err| {
Output.prettyErrorln("package.json failed to write due to error {s}", .{@errorName(err)});
Global.crash();
};
Expand Down Expand Up @@ -10075,6 +10127,9 @@ pub const PackageManager = struct {
&package_json_writer_two,
new_package_json,
&source,
.{
.indent = current_package_json_indent,
},
) catch |err| {
Output.prettyErrorln("package.json failed to write due to error {s}", .{@errorName(err)});
Global.crash();
Expand Down
1 change: 1 addition & 0 deletions src/install/lockfile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -4294,6 +4294,7 @@ pub const Package = extern struct {
) !WorkspaceEntry {
const workspace_json = try json_cache.getWithPath(allocator, log, abs_package_json_path, .{
.init_reset_store = false,
.guess_indentation = true,
}).unwrap();

const name_expr = workspace_json.root.get("name") orelse return error.MissingPackageName;
Expand Down
46 changes: 45 additions & 1 deletion src/js_lexer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const default_allocator = bun.default_allocator;
const C = bun.C;
const FeatureFlags = @import("feature_flags.zig");
const JavascriptString = []const u16;
const Indentation = bun.js_printer.Options.Indentation;

const unicode = std.unicode;

Expand Down Expand Up @@ -75,6 +76,8 @@ pub const JSONOptions = struct {
was_originally_macro: bool = false,

always_decode_escape_sequences: bool = false,

guess_indentation: bool = false,
};

pub fn decodeStringLiteralEscapeSequencesToUTF16(bytes: string, allocator: std.mem.Allocator) ![]const u16 {
Expand Down Expand Up @@ -102,6 +105,7 @@ pub fn NewLexer(
json_options.json_warn_duplicate_keys,
json_options.was_originally_macro,
json_options.always_decode_escape_sequences,
json_options.guess_indentation,
);
}

Expand All @@ -114,6 +118,7 @@ fn NewLexer_(
comptime json_options_json_warn_duplicate_keys: bool,
comptime json_options_was_originally_macro: bool,
comptime json_options_always_decode_escape_sequences: bool,
comptime json_options_guess_indentation: bool,
) type {
const json_options = JSONOptions{
.is_json = json_options_is_json,
Expand All @@ -124,6 +129,7 @@ fn NewLexer_(
.json_warn_duplicate_keys = json_options_json_warn_duplicate_keys,
.was_originally_macro = json_options_was_originally_macro,
.always_decode_escape_sequences = json_options_always_decode_escape_sequences,
.guess_indentation = json_options_guess_indentation,
};
return struct {
const LexerType = @This();
Expand Down Expand Up @@ -189,6 +195,16 @@ fn NewLexer_(
track_comments: bool = false,
all_comments: std.ArrayList(logger.Range),

indent_info: if (json_options.guess_indentation)
struct {
guess: Indentation = .{},
first_newline: bool = true,
}
else
void = if (json_options.guess_indentation)
.{}
else {},

pub fn clone(self: *const LexerType) LexerType {
return LexerType{
.log = self.log,
Expand Down Expand Up @@ -1211,8 +1227,36 @@ fn NewLexer_(
}
},
'\r', '\n', 0x2028, 0x2029 => {
lexer.step();
lexer.has_newline_before = true;

if (comptime json_options.guess_indentation) {
if (lexer.indent_info.first_newline and lexer.code_point == '\n') {
while (lexer.code_point == '\n' or lexer.code_point == '\r') {
lexer.step();
}

if (lexer.code_point != ' ' and lexer.code_point != '\t') {
// try to get the next one. this handles cases where the file starts
// with a newline
continue;
}

lexer.indent_info.first_newline = false;

const indent_character = lexer.code_point;
var count: usize = 0;
while (lexer.code_point == indent_character) {
lexer.step();
count += 1;
}

lexer.indent_info.guess.character = if (indent_character == ' ') .space else .tab;
lexer.indent_info.guess.scalar = count;
continue;
}
}

lexer.step();
continue;
},
'\t', ' ' => {
Expand Down
Loading

0 comments on commit 4fefb85

Please sign in to comment.