Skip to content

Commit 137cae9

Browse files
committed
Code quality pass: address review items
- Compute AssignmentBoundaryPrecedence from BinaryOperators dynamically - PathParametersBinding constant for "parameter" magic string - Token.ToString falls back to OriginalValue when Value is null - Move CA1859 suppression to .editorconfig (with explanation) - Structural equality for lists and besetning instances - Use BaconJsonSerializer.Options consistently in error responses - Document HTTP method case-sensitivity in LANGUAGE.md - Document BaconProcess.Closure semantics with XML comment - Add line numbers to RuntimeException with CLI integration - Add --host flag for binding to non-localhost addresses - Document HTTP-only and reverse proxy guidance
1 parent b14a9d8 commit 137cae9

18 files changed

Lines changed: 310 additions & 124 deletions

.editorconfig

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# CA1859: Use concrete types when possible
2+
# We deliberately return IReadOnlyList<T> for API stability over micro-optimization
3+
[*.cs]
4+
dotnet_diagnostic.CA1859.severity = none

LANGUAGE.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,13 @@ Run as web server with `--serve`:
166166
dotnet run --project src/Bacon.Cli/Bacon.Cli.csproj -- --serve api.bacon
167167
```
168168

169-
Currently supported HTTP methods: GET, POST, PUT, DELETE, PATCH.
169+
Options:
170+
- `--port <port>` — port to bind to (default 5000)
171+
- `--host <host>` — host to bind to (default localhost; use `0.0.0.0` for container scenarios)
172+
173+
**Notes:**
174+
- Bacon serves HTTP only. For HTTPS, use a reverse proxy like Caddy or nginx in front.
175+
- HTTP methods are case-sensitive and must be uppercase. Lowercase `get` will be parsed as an identifier, not an HTTP method.
170176

171177
## Comments
172178

TODO.md

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,8 @@
11
# TODO
22

3-
Items found during code review with Claude Cowork (2026-05-03).
4-
5-
## Bugs to fix
6-
7-
All bugs from initial code review have been fixed.
8-
93
## Code quality
104

11-
- [ ] Magic precedence number `31` in `Parser.Statements.cs` — compute as `BinaryOperators[TokenType.Is].Precedence + 1`
12-
- [ ] `AreEqual` returns false for structurally-identical lists/besetnings — extend to compare elements/fields
13-
- [ ] `BaconProcess.Closure` always captures `_global`; rename or document that nested process declarations aren't supported
14-
- [ ] HTTP method tokens are case-sensitive in lexer; either fold to uppercase or document explicitly
15-
- [ ] `BaconWebHost` always binds to localhost; consider `--host` flag for container scenarios
16-
- [ ] `RuntimeException` carries no source location; add `(string message, int line)` constructor
17-
- [ ] Magic string `"parameter"` in `Evaluator.Web.cs` should be a named constant
18-
- [ ] `Token.ToString()` drops `OriginalValue` for tokens with null `Value`
19-
- [ ] CA1859 suppressions are scattered across files; consolidate via `.editorconfig`
20-
- [ ] `WriteJsonResponse` mixes manual content-type with `WriteAsJsonAsync` style elsewhere
5+
All items from initial code review have been addressed.
216

227
## Future features
238

@@ -29,3 +14,4 @@ All bugs from initial code review have been fixed.
2914
- More stdlib functions
3015
- Package as `dotnet tool` for direct `bacon` command
3116
- Module system (currently `import` is a no-op)
17+
- HTTPS support (currently HTTP-only)

examples/runtime_error.bacon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
prosess hovedprogram() {
2+
fast x er 5
3+
leverer x + "hello"
4+
}

src/Bacon.Cli/Program.cs

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,31 @@
1010
}
1111

1212
var serve = args.Contains("--serve");
13-
var portArg = ParsePortArg(args);
13+
14+
int? portArg;
15+
string? hostArg;
16+
try
17+
{
18+
portArg = ParsePortArg(args);
19+
hostArg = ParseHostArg(args);
20+
}
21+
catch (ArgumentException ex)
22+
{
23+
Console.Error.WriteLine($"Error: {ex.Message}");
24+
PrintUsage();
25+
return 1;
26+
}
27+
1428
var filePath = ParseFilePath(args);
1529

16-
if (portArg.HasValue && !serve)
30+
if ((portArg.HasValue || hostArg != null) && !serve)
1731
{
18-
Console.Error.WriteLine("Error: --port can only be used with --serve");
32+
Console.Error.WriteLine("Error: --port and --host can only be used with --serve");
1933
PrintUsage();
2034
return 1;
2135
}
2236

23-
if (filePath == null)
37+
if (string.IsNullOrWhiteSpace(filePath))
2438
{
2539
Console.Error.WriteLine("Error: missing file argument");
2640
PrintUsage();
@@ -33,7 +47,6 @@
3347
return 1;
3448
}
3549

36-
var port = portArg ?? 5000;
3750
var source = await File.ReadAllTextAsync(filePath);
3851

3952
try
@@ -44,7 +57,7 @@
4457
if (serve)
4558
{
4659
var host = new BaconWebHost(program);
47-
await host.RunAsync(port);
60+
await host.RunAsync(portArg ?? 5000, hostArg ?? "localhost");
4861
}
4962
else
5063
{
@@ -65,7 +78,9 @@
6578
}
6679
catch (RuntimeException ex)
6780
{
68-
Console.Error.WriteLine($"Runtime error: {ex.Message}");
81+
Console.Error.WriteLine(ex.Line.HasValue
82+
? $"Runtime error at {filePath}:{ex.Line}: {ex.Message}"
83+
: $"Runtime error: {ex.Message}");
6984
return 1;
7085
}
7186

@@ -84,23 +99,33 @@
8499
return port is < 1 or > 65535 ? throw new ArgumentException($"Invalid port value: {port} (must be 1-65535)") : port;
85100
}
86101

102+
static string? ParseHostArg(string[] args)
103+
{
104+
var index = Array.IndexOf(args, "--host");
105+
if (index < 0) return null;
106+
107+
return index + 1 >= args.Length ? throw new ArgumentException("--host requires a value") : args[index + 1];
108+
}
109+
87110
static string? ParseFilePath(string[] args)
88111
{
89112
for (var i = 0; i < args.Length; i++)
90113
{
91-
if (!args[i].StartsWith("--"))
114+
if (!args[i].StartsWith("--", StringComparison.Ordinal))
92115
return args[i];
93-
if (args[i] == "--port")
116+
117+
if (args[i] is "--port" or "--host")
94118
i++;
95119
}
96120
return null;
97121
}
98122

99123
static void PrintUsage()
100124
{
101-
Console.Error.WriteLine("Usage: Bacon.Cli [--serve] [--port <port>] <file.bacon>");
125+
Console.Error.WriteLine("Usage: Bacon.Cli [--serve] [--port <port>] [--host <host>] <file.bacon>");
102126
Console.Error.WriteLine();
103127
Console.Error.WriteLine("Options:");
104128
Console.Error.WriteLine(" --serve Run as a web server");
105129
Console.Error.WriteLine(" --port <port> Port to serve on (default 5000, requires --serve)");
130+
Console.Error.WriteLine(" --host <host> Host to bind to (default localhost, requires --serve)");
106131
}

src/Bacon.Compiler/Evaluation/BaconValue.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@ public sealed record BaconBesetningInstance(
2121
}
2222
public sealed record BaconBesetningType(BesetningDeclaration Declaration) : BaconValue;
2323

24-
public sealed record BaconProcess(
25-
ProcessDeclaration Declaration,
26-
Environment Closure) : BaconValue;
24+
/// <summary>
25+
/// The environment captured at process declaration time. Currently, always _global
26+
/// because Bacon doesn't yet support nested process declarations. When that changes,
27+
/// this will capture the lexical scope at declaration site.
28+
/// </summary>
29+
public sealed record BaconProcess(ProcessDeclaration Declaration, Environment Closure) : BaconValue;
2730

2831
public sealed record BaconBuiltinFunction(
2932
string Name,

src/Bacon.Compiler/Evaluation/Evaluator.Calls.cs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,24 @@ private BaconValue EvaluateCall(CallExpression call)
1111

1212
return callee switch
1313
{
14-
BaconProcess process => CallProcess(process, args),
14+
BaconProcess process => CallProcess(process, args, call.Line),
1515
BaconBuiltinFunction builtin => builtin.Implementation(args),
16-
BaconBesetningType type => InstantiateBesetning(type, args),
17-
_ => throw new RuntimeException($"Cannot call value of type {TypeName(callee)}")
16+
BaconBesetningType type => InstantiateBesetning(type, args, call.Line),
17+
_ => throw new RuntimeException($"Cannot call value of type {TypeName(callee)}", call.Line)
1818
};
1919
}
2020

21-
[System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1859",
22-
Justification = "We do not need to modify the list just read it")]
2321
private static BaconBesetningInstance InstantiateBesetning(
2422
BaconBesetningType type,
25-
IReadOnlyList<BaconValue> args)
23+
IReadOnlyList<BaconValue> args,
24+
int line)
2625
{
2726
var decl = type.Declaration;
2827

2928
if (args.Count != decl.Fields.Count)
3029
{
3130
throw new RuntimeException(
32-
$"'{decl.Name}' expected {decl.Fields.Count} arguments, got {args.Count}");
31+
$"'{decl.Name}' expected {decl.Fields.Count} arguments, got {args.Count}", line);
3332
}
3433

3534
var fields = new Dictionary<string, BaconValue>();
@@ -41,16 +40,14 @@ private static BaconBesetningInstance InstantiateBesetning(
4140
return new BaconBesetningInstance(type, fields);
4241
}
4342

44-
[System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1859",
45-
Justification = "We do not need to modify the list just read it")]
46-
private BaconValue CallProcess(BaconProcess process, IReadOnlyList<BaconValue> args)
43+
private BaconValue CallProcess(BaconProcess process, IReadOnlyList<BaconValue> args, int line)
4744
{
4845
var decl = process.Declaration;
4946

5047
if (args.Count != decl.Parameters.Count)
5148
{
5249
throw new RuntimeException(
53-
$"'{decl.Name}' expected {decl.Parameters.Count} arguments, got {args.Count}");
50+
$"'{decl.Name}' expected {decl.Parameters.Count} arguments, got {args.Count}", line);
5451
}
5552

5653
var callEnv = new Environment(parent: process.Closure);

0 commit comments

Comments
 (0)