ClaudeReports/GDScript-formatter_20260616.html at master · qarmin/ClaudeReports · GitHub

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1">
<title>Code Audit — Interactive</title>
<style>
*,*::before,*::after{box-sizing:border-box;margin:0;padding:0}
body{font-family:-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,sans-serif;background:#0f172a;color:#e2e8f0;font-size:14px;line-height:1.5}
.app{max-width:1100px;margin:0 auto;padding:1.5rem 1rem}
.top{display:flex;justify-content:space-between;align-items:flex-start;gap:1rem;margin-bottom:1rem;flex-wrap:wrap}
h1{font-size:1.5rem;font-weight:700;color:#f1f5f9}
.meta{font-size:.8rem;color:#64748b;margin-top:.2rem}
.top-right{display:flex;align-items:center;gap:.75rem;flex-wrap:wrap}
.project-info{margin-bottom:1rem;padding:.75rem .9rem;background:#1e293b;border:1px solid #334155;border-radius:8px}
.project-info .pi-name{font-size:.95rem;font-weight:700;color:#f1f5f9}
.project-info .pi-desc{font-size:.82rem;color:#cbd5e1;margin-top:.25rem}
.project-info .pi-row{display:flex;flex-wrap:wrap;gap:.4rem .9rem;margin-top:.45rem;font-size:.78rem;color:#94a3b8}
.project-info .pi-row .pi-k{color:#64748b}
.project-info a{color:#60a5fa;text-decoration:none}
.project-info a:hover{text-decoration:underline}
.project-info code{font-family:"JetBrains Mono","Fira Code",Consolas,monospace;font-size:.74rem;color:#93c5fd}
.pi-repo-btn{display:inline-flex;align-items:center;gap:.3rem;padding:.18rem .5rem;background:#0f172a;border:1px solid #3b82f6;border-radius:6px;color:#60a5fa !important;font-weight:600}
.pi-repo-btn:hover{background:#1d283a;text-decoration:none !important}
#search{background:#1e293b;border:1px solid #334155;color:#e2e8f0;padding:.4rem .75rem;border-radius:6px;font-size:.875rem;width:220px;outline:none}
#search:focus{border-color:#3b82f6}
.group-label{font-size:.8rem;color:#94a3b8;display:flex;align-items:center;gap:.4rem}
select{background:#1e293b;border:1px solid #334155;color:#e2e8f0;padding:.35rem .6rem;border-radius:6px;font-size:.8rem;outline:none;cursor:pointer}
select:focus{border-color:#3b82f6}
.filterbar{display:flex;flex-direction:column;gap:.4rem;margin-bottom:.6rem;padding:.6rem .75rem;background:#1e293b;border:1px solid #334155;border-radius:8px}
.filter-row{display:flex;flex-wrap:wrap;gap:.3rem;align-items:center}
.filter-lbl{font-size:.65rem;font-weight:700;text-transform:uppercase;letter-spacing:.08em;color:#475569;white-space:nowrap;margin-right:.25rem}
.filter-toggle{font-size:.65rem;font-weight:600;padding:.18rem .5rem;border-radius:4px;border:1px solid #334155;background:#0f172a;color:#94a3b8;cursor:pointer;margin-left:auto;white-space:nowrap;letter-spacing:.04em}
.filter-toggle:hover{background:#1e293b;color:#e2e8f0;border-color:#475569}
.badge{font-size:.68rem;font-weight:700;padding:.18rem .5rem;border-radius:4px;border:1px solid transparent;cursor:pointer;letter-spacing:.03em;transition:opacity .12s,filter .12s;user-select:none}
.badge.off{opacity:.28;filter:grayscale(.5)}
.sev-critical{background:#450a0a;color:#fca5a5;border-color:#991b1b}
.sev-high    {background:#431407;color:#fdba74;border-color:#9a3412}
.sev-medium  {background:#422006;color:#fcd34d;border-color:#854d0e}
.sev-low     {background:#0c1a3a;color:#93c5fd;border-color:#1d4ed8}
.sev-negligible{background:#1e293b;color:#64748b;border-color:#334155}
.type-badge{background:#0c1e33;color:#7dd3fc;border-color:#0369a1}
.easy-badge{background:#052e16;color:#86efac;border-color:#16a34a}
.st-open{background:#1e293b;color:#475569;border-color:#334155}
.st-fixed{background:#052e16;color:#4ade80;border-color:#16a34a}
.st-wont{background:#431407;color:#fdba74;border-color:#9a3412}
.st-fp{background:#2e1065;color:#c4b5fd;border-color:#7c3aed}
#progress-wrap{display:flex;align-items:center;gap:.75rem;margin-bottom:.6rem;flex-wrap:wrap}
#progress-bar{flex:1;min-width:120px;height:7px;background:#1e293b;border-radius:4px;overflow:hidden;border:1px solid #334155}
#progress-fill{height:100%;background:#22c55e;border-radius:4px;transition:width .35s ease;width:0}
#progress-text{font-size:.75rem;color:#64748b;white-space:nowrap}
.action-btn{font-size:.72rem;padding:.25rem .6rem;border-radius:5px;border:1px solid #334155;background:#1e293b;color:#94a3b8;cursor:pointer;white-space:nowrap}
.action-btn:hover{background:#334155;color:#e2e8f0}
#hidden-bar{margin-bottom:.6rem}
.hidden-inner{display:flex;flex-wrap:wrap;gap:.35rem;align-items:center}
.hidden-lbl{font-size:.72rem;color:#475569}
.show-btn{font-size:.7rem;padding:.15rem .45rem;border-radius:4px;background:#1e293b;color:#94a3b8;border:1px solid #334155;cursor:pointer}
.show-btn:hover{background:#334155;color:#e2e8f0}
.show-all-btn{font-size:.7rem;padding:.15rem .45rem;border-radius:4px;background:#2563eb;color:#fff;border:none;cursor:pointer}
.show-all-btn:hover{background:#1d4ed8}
.group-section{margin-bottom:1.5rem}
.group-header{display:flex;align-items:center;gap:.5rem;margin-bottom:.5rem;padding-bottom:.35rem;border-bottom:1px solid #334155}
.group-title{font-size:.95rem;font-weight:700;color:#cbd5e1}
.group-title.all-done{color:#4ade80}
.group-count{font-size:.72rem;color:#475569;background:#1e293b;padding:.1rem .4rem;border-radius:10px;white-space:nowrap}
.icon-btn{background:transparent;border:none;color:#334155;cursor:pointer;font-size:.85rem;padding:.05rem .2rem;line-height:1}
.icon-btn:hover{color:#64748b}
.hide-btn{margin-left:auto;font-size:.68rem;padding:.12rem .45rem;border-radius:4px;background:transparent;color:#334155;border:1px solid #1e293b;cursor:pointer}
.hide-btn:hover{background:#1e293b;color:#64748b}
.card{background:#1e293b;border:1px solid #334155;border-radius:8px;margin-bottom:.6rem;overflow:hidden;transition:opacity .15s}
.card.resolved{opacity:.55;border-left:3px solid #16a34a}
.card.resolved:hover{opacity:.85}
.card-header{display:flex;align-items:center;gap:.5rem;padding:.45rem .85rem;background:#162032;border-bottom:1px solid #334155;flex-wrap:wrap;cursor:pointer;user-select:none}
.card-header:hover{background:#1a2840}
.card-arrow{font-size:.65rem;color:#334155;flex-shrink:0}
.card-id{font-size:.75rem;font-weight:700;color:#64748b;font-family:monospace;letter-spacing:.04em}
.card-loc{font-size:.72rem;color:#64748b;font-family:"JetBrains Mono","Fira Code",Consolas,monospace;letter-spacing:.02em;white-space:nowrap;overflow:hidden;text-overflow:ellipsis;min-width:0}
.status-btn{margin-left:auto;font-size:.68rem;font-weight:600;padding:.15rem .5rem;border-radius:4px;border:1px solid transparent;cursor:pointer;white-space:nowrap;transition:filter .1s}
.status-btn:hover{filter:brightness(1.25)}
.copy-btn{font-size:.68rem;font-weight:600;padding:.15rem .5rem;border-radius:4px;border:1px solid #334155;background:#0f172a;color:#94a3b8;cursor:pointer;white-space:nowrap;transition:background .12s,color .12s}
.copy-btn:hover{background:#334155;color:#e2e8f0}
.card-body{padding:.85rem .9rem}
.field-lbl{font-size:.85rem;font-weight:700;text-transform:uppercase;letter-spacing:.1em;color:#7dd3fc;margin:1rem 0 .35rem;padding-bottom:.18rem;border-bottom:1px solid #1e293b}
.field-lbl:first-child{margin-top:0}
.desc,.fix-txt{font-size:.875rem;color:#cbd5e1;line-height:1.55}
.desc p,.fix-txt p{margin:.4rem 0}
.desc p:first-child,.fix-txt p:first-child{margin-top:0}
.desc p:last-child,.fix-txt p:last-child{margin-bottom:0}
.loc-block{margin:.3rem 0 .6rem}
.loc-ref{font-size:.8rem;color:#38bdf8;font-family:"JetBrains Mono","Fira Code",Consolas,monospace;font-weight:600;margin-bottom:.22rem}
.loc-lines{color:#475569;font-weight:400}
pre{background:#0f172a;color:#cdd6f4;border-radius:5px;padding:.6rem .8rem;overflow-x:auto;white-space:pre;font-size:.8rem;line-height:1.55;font-family:"JetBrains Mono","Fira Code",Consolas,monospace;border:1px solid #1e293b;max-height:320px}
.desc pre.md-code,.fix-txt pre.md-code{margin:.5rem 0;font-size:.82rem;color:#e2e8f0;background:#0b1224;border:1px solid #1e293b;max-height:none}
code.md-inline{font-family:"JetBrains Mono","Fira Code",Consolas,monospace;font-size:.86em;background:#0b1224;color:#fca5a5;border:1px solid #1e293b;border-radius:3px;padding:.04rem .32rem;white-space:pre-wrap;word-break:break-word}
.not-found{font-size:.8rem;color:#334155;font-style:italic}
mark{background:#854d0e;color:#fef9c3;border-radius:2px;padding:0 .1rem}
.no-results{text-align:center;color:#334155;padding:3rem 1rem;font-size:.9rem}
</style>
</head>
<body>
<script type="application/json" id="D">[{"id":"PANIC_1","type":"panic","type_label":"Panic","severity":"critical","easy":true,"description":"Out-of-bounds index access at `self.content.as_bytes()[byte_idx - 1]` in `handle_two_blank_line`.\n`byte_idx` is walked backward until it hits 0 OR finds a newline, so it CAN be 0 when the\nblock to insert before is at the very start of the file.  The condition then reads\n`self.content.as_bytes()[0 - 1]` which is an arithmetic underflow on `usize`,\ncausing a panic at runtime.  Any valid GDScript file whose very first token triggers\none of the two tree-sitter queries (e.g. a file that starts with a function definition\nimmediately followed by a variable) will reproduce the panic.","fix":"Guard against byte_idx == 0 before the subtraction:\n\n    if byte_idx == 0\n        || !(self.content.as_bytes()[byte_idx] == b'\\n'\n            && self.content.as_bytes()[byte_idx - 1] == b'\\n')\n    {","locations":[{"ref":"src/formatter.rs:690-691","code":"  690 |             if !(self.content.as_bytes()[byte_idx] == b'\\n'\n  691 |                 && self.content.as_bytes()[byte_idx - 1] == b'\\n')","lines":2}]},{"id":"PANIC_2","type":"panic","type_label":"Panic","severity":"high","easy":true,"description":"`TopiaryQuery::new(...)` on line 97 is `.unwrap()`-ed.  If the bundled `queries/gdscript.scm`\nquery file is malformed (e.g. after an update to the tree-sitter grammar changes node\nnames), the call panics instead of returning a structured error to the caller.\nSimilarly `parser.set_language(...)` on line 65 is `.unwrap()`-ed.","fix":"Propagate through `?` after converting with `.map_err(|e| format!(...))`.\n`Formatter::new` would need to return `Result<Self, ...>`.","locations":[{"ref":"src/formatter.rs:65","code":"   65 |             .unwrap();","lines":1},{"ref":"src/formatter.rs:97","code":"   97 |             query: TopiaryQuery::new(&tree_sitter_gdscript::LANGUAGE.into(), QUERY).unwrap(),","lines":1}]},{"id":"PANIC_3","type":"panic","type_label":"Panic","severity":"high","easy":true,"description":"`parser.parse()` returns `Option<Tree>` and is unwrapped unconditionally in at least six\nplaces: `Formatter::new` (line 66), `postprocess` (line 169), `postprocess_tree_sitter`\n(line 501), `fix_nested_parenthesized_lambda_indentation` (line 459),\n`reorder` (line 131), `ensure_safe_reorder` (line 203), and\n`parse_top_level_token_signatures` (line 769 uses `ok_or` which is fine, but the\nothers use `.unwrap()`).  Tree-sitter's `parse()` returns `None` when the parse is\ncancelled via a timeout/cancellation flag or when the language is not set, even though\nthe language is set. A stale/very large file or an internal tree-sitter error would\ncause an unrecoverable panic instead of a graceful error.","fix":"Replace each `.unwrap()` on `parser.parse(...)` with `.ok_or(\"tree-sitter parse failed\")?`\nand propagate the error up.  Functions that currently return `&mut Self` will need to\nreturn `Result<&mut Self, ...>` or at minimum use `expect` with a descriptive message\nif the intent is that the error is truly impossible.","locations":[{"ref":"src/formatter.rs:66","code":"   66 |         let tree = parser.parse(&content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:131","code":"  131 |         self.tree = self.parser.parse(&self.content, Some(&self.tree)).unwrap();","lines":1},{"ref":"src/formatter.rs:169","code":"  169 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:203","code":"  203 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:459","code":"  459 |         self.tree = self.parser.parse(&self.content, Some(&self.tree)).unwrap();","lines":1},{"ref":"src/formatter.rs:501","code":"  501 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:579","code":"  579 |         self.tree = self.parser.parse(&self.content, Some(&self.tree)).unwrap();","lines":1}]},{"id":"PANIC_4","type":"panic","type_label":"Panic","severity":"high","easy":true,"description":"In `handle_two_blank_line`, after the while loop that walks backward to find a newline, `byte_idx`\ncan be 0 (when the first declaration appears at the very start of the file with no preceding newline).\nThe subsequent condition on lines 690-691 then evaluates `self.content.as_bytes()[byte_idx - 1]` where\n`byte_idx` is 0, causing a usize underflow and an index-out-of-bounds panic in both debug and release builds.","fix":"Add a guard before the condition so the second byte access is only attempted when `byte_idx > 0`:\n\n    if !(byte_idx > 0\n        && self.content.as_bytes()[byte_idx] == b'\\n'\n        && self.content.as_bytes()[byte_idx - 1] == b'\\n')\n    {","locations":[{"ref":"src/formatter.rs:663-691","code":"  663 |                     let mut byte_idx = insert_before.start_byte();\n  664 |                     let mut position = insert_before.start_position();\n  665 |                     position.column = 0;\n  666 |                     while byte_idx > 0 && self.content.as_bytes()[byte_idx] != b'\\n' {\n  667 |                         byte_idx -= 1;\n  668 |                     }\n  669 |                     new_lines_at.push((byte_idx, position));\n  670 |                 }\n  671 |             };\n  672 | \n  673 |         // First we need to find all the places where we should add blank lines.\n  674 |         // We can't modify the content string while tree-sitter is borrowing it, so we\n  675 |         // collect all the positions first, then make changes afterward.\n  676 |         let mut new_lines_at = Vec::new();\n  677 | \n  678 |         for query_str in &queries {\n  679 |             process_query(query_str, &mut new_lines_at);\n  680 |         }\n  681 | \n  682 |         // We sort the positions in reverse order so that when we insert new lines,\n  683 |         // we don't mess up the positions of the other insertions we need to make.\n  684 |         new_lines_at.sort_by(|a, b| b.cmp(a));\n  685 | \n  686 |         for (byte_idx, position) in new_lines_at {\n  687 |             let mut new_end_position = position;\n  688 |             let mut new_end_byte_idx = byte_idx;\n  689 |             // Only add a second blank line if there isn't already one\n  690 |             if !(self.content.as_bytes()[byte_idx] == b'\\n'\n  691 |                 && self.content.as_bytes()[byte_idx - 1] == b'\\n')","lines":29}]},{"id":"PANIC_5","type":"panic","type_label":"Panic","severity":"high","easy":true,"description":"In `handle_two_blank_line`, the condition at line 691 reads `self.content.as_bytes()[byte_idx - 1]`. When `byte_idx == 0` (which happens when the first top-level definition starts at byte 0, e.g. a file with no leading blank line or with a leading newline at position 0) AND `content[0] == '\\n'`, the short-circuit for `&&` does not fire (the first condition IS true), so `byte_idx - 1` underflows from 0 to `usize::MAX`, causing an out-of-bounds panic. The while loop on line 666 stops when `byte_idx > 0` is false, leaving `byte_idx == 0`. If the file content starts with `\\n`, the first condition is true and the second is evaluated on `usize::MAX`.","fix":"Guard the second check: `if byte_idx == 0 || !(self.content.as_bytes()[byte_idx] == b'\\n' && self.content.as_bytes()[byte_idx - 1] == b'\\n')`.","locations":[{"ref":"src/formatter.rs:690-691","code":"  690 |             if !(self.content.as_bytes()[byte_idx] == b'\\n'\n  691 |                 && self.content.as_bytes()[byte_idx - 1] == b'\\n')","lines":2}]},{"id":"PANIC_6","type":"panic","type_label":"Panic","severity":"high","easy":false,"description":"Direct index `variable_node.children[0]` on line 1038 panics if `variable_statement`\nhas no children in the `GdTree`.  A malformed or partially-parsed source file can\nproduce nodes with zero children.  The same pattern appears for `child.children[0]`\non line 1017 inside `move_annotations` - it is guarded only by `child.grammar_name ==\n\"annotation\"`, not by whether the annotation node actually has children.","fix":"Replace direct index accesses with `.get(0)` and return early / `return None` if the\nresult is `None`:\n    let Some(&first_child_id) = child.children.first() else { return None; };\n    let variable_first_child_id = match variable_node.children.first() {\n        Some(&id) => id,\n        None => continue,\n    };","locations":[{"ref":"src/formatter.rs:1017","code":" 1017 |                             let Some(annotation_name) = &self.nodes[child.children[0]].text else {","lines":1},{"ref":"src/formatter.rs:1038","code":" 1038 |                     let variable_first_child_id = variable_node.children[0];","lines":1}]},{"id":"PANIC_7","type":"panic","type_label":"Panic","severity":"medium","easy":false,"description":"`regex_replace_all_outside_strings_or_comments` calls `.unwrap()` on\n`descendant_for_byte_range(start_byte, start_byte)` (line 533-535). `descendant_for_byte_range`\nreturns `Option<Node>` and can return `None` when the requested byte offset falls outside the range\nof the root node, for example when the tree-sitter tree is out of sync with the content after\nincremental edits. A `None` result here panics.","fix":"Replace `.unwrap()` with a guard:\n\n    let Some(node) = self.tree.root_node()\n        .descendant_for_byte_range(start_byte, start_byte)\n    else {\n        continue;\n    };","locations":[{"ref":"src/formatter.rs:530-535","code":"  530 |             let old_end_byte = m.end();\n  531 |             let node = self\n  532 |                 .tree\n  533 |                 .root_node()\n  534 |                 .descendant_for_byte_range(start_byte, start_byte)\n  535 |                 .unwrap();","lines":6}]},{"id":"PANIC_8","type":"panic","type_label":"Panic","severity":"medium","easy":false,"description":"Inside the `process_query` closure in `handle_two_blank_line`, a `Query::new` failure is handled with an unconditional `panic!` rather than propagating an error. If the tree-sitter-gdscript grammar is updated so that any node kind used in the hardcoded query strings no longer exists, this panic will be triggered at runtime for every formatted file. The function signature already returns `&mut Self`, but the enclosing method uses `Result`-returning callers, so the error could be propagated.","fix":"Change the process_query closure to return a `Result`, propagate `Query::new` errors up through `handle_two_blank_line` (change return type to `Result<&mut Self, ...>`), and handle the failure gracefully instead of panicking.","locations":[{"ref":"src/formatter.rs:606-614","code":"  606 |                 let query = match Query::new(\n  607 |                     &tree_sitter::Language::new(tree_sitter_gdscript::LANGUAGE),\n  608 |                     query_str,\n  609 |                 ) {\n  610 |                     Ok(q) => q,\n  611 |                     Err(err) => {\n  612 |                         panic!(\"Failed to create query: {}\", err);\n  613 |                     }\n  614 |                 };","lines":9}]},{"id":"PANIC_9","type":"panic","type_label":"Panic","severity":"medium","easy":false,"description":"`GdTree::move_annotations` accesses `child.children[0]` without checking whether the annotation node\nhas any children (line 1017). If tree-sitter produces an annotation node with an empty children list\n(possible with malformed or error-recovering input), the index access panics.","fix":"Guard the access: replace `child.children[0]` with `child.children.first()` and handle the `None`\ncase:\n\n    let Some(&first_child_id) = child.children.first() else {\n        return None;\n    };\n    let Some(annotation_name) = &self.nodes[first_child_id].text else {\n        return None;\n    };","locations":[{"ref":"src/formatter.rs:1009-1029","code":" 1009 |                     let annotations_to_move = (0..index)\n 1010 |                         .rev()\n 1011 |                         .map_while(|i| {\n 1012 |                             let child_id = self.nodes[parent_id].children[i];\n 1013 |                             let child = &self.nodes[child_id];\n 1014 |                             if child.grammar_name != \"annotation\" {\n 1015 |                                 return None;\n 1016 |                             }\n 1017 |                             let Some(annotation_name) = &self.nodes[child.children[0]].text else {\n 1018 |                                 return None;\n 1019 |                             };\n 1020 |                             if annotation_name != \"onready\" && annotation_name != \"export\" {\n 1021 |                                 return None;\n 1022 |                             }\n 1023 |                             let parent = &mut self.nodes[parent_id];\n 1024 |                             // When we found one, we remove it from the parent and collect them in a vector\n 1025 |                             let annotation_id = parent.children.remove(i);\n 1026 |                             index -= 1;\n 1027 |                             Some(annotation_id)\n 1028 |                         })\n 1029 |                         .collect::<Vec<_>>();","lines":21}]},{"id":"PANIC_10","type":"panic","type_label":"Panic","severity":"medium","easy":false,"description":"In `GdTree::move_annotations`, line 1038 performs an unchecked direct index `variable_node.children[0]`. A `variable_statement` node with ERROR recovery in tree-sitter can lack named children, making `.children` empty. `GdTree` only stores named children (line 907 skips anonymous nodes), so any malformed variable statement without a named `name` child causes an index-out-of-bounds panic. The function is called on the input tree (potentially malformed user code) in `validate_formatting`.","fix":"Replace `variable_node.children[0]` with `variable_node.children.first().copied()` and handle the `None` case gracefully (e.g. `continue`).","locations":[{"ref":"src/formatter.rs:1037-1038","code":" 1037 |                     let variable_node = &self.nodes[child_id];\n 1038 |                     let variable_first_child_id = variable_node.children[0];","lines":2}]},{"id":"PANIC_11","type":"panic","type_label":"Panic","severity":"medium","easy":false,"description":"`io::stdout().flush().unwrap()` in main.rs will panic if stdout flushing fails (e.g., when\nstdout is a broken pipe). This is in the hot path when formatting files.","fix":"Replace with `.map_err(|e| format!(\"Failed to flush stdout: {}\", e))?;` and propagate\nthe error, or at minimum use `.ok()` if flushing is truly best-effort.","locations":[{"ref":"src/main.rs:219","code":"  219 |     io::stdout().flush().unwrap();","lines":1}]},{"id":"PANIC_12","type":"panic","type_label":"Panic","severity":"low","easy":false,"description":"In make_release.rs, `io::stdout().flush().expect(\"Failed to flush stdout\")` and\n`io::stdin().read_line(&mut input).expect(\"Failed to read user input\")` will panic\non I/O failure. While this is a dev-only script and panics are somewhat acceptable,\nthe `.expect` calls will produce an abrupt unformatted backtrace rather than the\nclean `print_error_and_exit` path used everywhere else in the same file.","fix":"Use `unwrap_or_else(|e| print_error_and_exit(&format!(\"...\")))` consistently, matching\nthe pattern already used in `run()`.","locations":[{"ref":"src/bin/make_release.rs:13","code":"   13 |     io::stdout().flush().expect(\"Failed to flush stdout\");","lines":1},{"ref":"src/bin/make_release.rs:17","code":"   17 |         .expect(\"Failed to read user input\");","lines":1}]},{"id":"PANIC_13","type":"panic","type_label":"Panic","severity":"low","easy":false,"description":"`parser.parse()` returns `Option<Tree>` and can return `None` when parsing times out or fails due\nto resource exhaustion. There are seven `.unwrap()` call sites on this return value across\n`formatter.rs` (lines 66, 131, 169, 187, 203, 459, 501, 579). In practice tree-sitter only returns\n`None` on explicit timeout or cancellation (neither of which is configured here), but the panics are\nunhandled.","fix":"Replace each `.unwrap()` with `ok_or(\"Failed to parse GDScript\")?` and propagate the error, or\nuse `.expect()` with a descriptive message so failures are at least diagnosed clearly instead of\nproducing a bare index-OOB panic.","locations":[{"ref":"src/formatter.rs:65-66","code":"   65 |             .unwrap();\n   66 |         let tree = parser.parse(&content, None).unwrap();","lines":2},{"ref":"src/formatter.rs:131","code":"  131 |         self.tree = self.parser.parse(&self.content, Some(&self.tree)).unwrap();","lines":1},{"ref":"src/formatter.rs:169","code":"  169 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:187","code":"  187 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:203","code":"  203 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:459","code":"  459 |         self.tree = self.parser.parse(&self.content, Some(&self.tree)).unwrap();","lines":1},{"ref":"src/formatter.rs:501","code":"  501 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":1},{"ref":"src/formatter.rs:579","code":"  579 |         self.tree = self.parser.parse(&self.content, Some(&self.tree)).unwrap();","lines":1}]},{"id":"PANIC_14","type":"panic","type_label":"Panic","severity":"low","easy":true,"description":"src/main.rs:219 calls `io::stdout().flush().unwrap()`. If stdout is connected to a broken pipe (e.g. the program is started and the receiving process closes stdin before the flush completes), this panics instead of exiting cleanly.","fix":"Replace `.unwrap()` with `.ok()` or handle the error with a graceful exit: `if let Err(e) = io::stdout().flush() { eprintln!(\"...\"); std::process::exit(1); }`","locations":[{"ref":"src/main.rs:219","code":"  219 |     io::stdout().flush().unwrap();","lines":1}]},{"id":"PANIC_15","type":"panic","type_label":"Panic","severity":"low","easy":true,"description":"is_static_method uses unwrap_or(\"\") to silently swallow a UTF-8 decoding error from tree-sitter (line 824). All other callers propagate the error via ?. An encoding error here causes the function to return false (treating the method as non-static), silently misclassifying it rather than surfacing the error.","fix":"Change the signature to return Result<bool, ...>, use ?, and propagate the error. Alternatively accept a pre-extracted &str to avoid re-extraction entirely.","locations":[{"ref":"src/reorder.rs:823-825","code":"  823 | fn is_static_method(node: Node, content: &str) -> bool {\n  824 |     let text = node.utf8_text(content.as_bytes()).unwrap_or(\"\");\n  825 |     text.contains(\"static func\")","lines":3}]},{"id":"OVF_1","type":"overflow_underflow","type_label":"Overflow Underflow","severity":"high","easy":false,"description":"`last_node.start_position().row - 1` on line 633 performs unchecked subtraction on\n`usize`.  `start_position().row` is of type `usize`.  If `last_node` is on row 0\n(i.e., the entire file is a single line and the comment/node is at the very start),\nthis underflows to `usize::MAX`, which means the comparison against\n`last_comment_node.start_position().row` will never match (harmless wrong behaviour)\nbut is still undefined-wrapping-arithmetic under `debug_assertions`.\n\nSimilarly, `m.captures[comment_node_index].node.start_position().row - 1` on line 655\ncan underflow when `comment_node_index` refers to a node on row 0.","fix":"Use saturating_sub(1) or guard with a preceding check:\n    last_node.start_position().row.checked_sub(1).map_or(false, |r| {\n        last_comment_node.start_position().row == r\n    })","locations":[{"ref":"src/formatter.rs:633","code":"  633 |                                 == last_node.start_position().row - 1;","lines":1},{"ref":"src/formatter.rs:655","code":"  655 |                                     == m.captures[comment_node_index].node.start_position().row - 1","lines":1}]},{"id":"LOGIC_1","type":"logic_error","type_label":"Logic Error","severity":"high","easy":true,"description":"lint_files_pretty stores per-file issues in a HashMap<String, Vec<_>> and then iterates it for output. HashMap iteration order is non-deterministic, so the file output order changes between runs. lint_files_standard processes files in the deterministic input order (sorted by find_gdscript_files), but the pretty mode does not. This makes --pretty output unreproducible for multi-file linting.","fix":"Replace `HashMap<String, Vec<_>>` with a `Vec<(String, Vec<LintIssue>)>` that preserves the original file iteration order, or sort file_issues.keys() before printing.","locations":[{"ref":"src/linter.rs:186-203","code":"  186 |         let mut file_issues: HashMap<String, Vec<_>> = HashMap::new();\n  187 |         let mut has_issues = false;\n  188 | \n  189 |         for file_path in gdscript_files {\n  190 |             let source_code = fs::read_to_string(file_path).map_err(|error| {\n  191 |                 format!(\"Failed to read file {}: {}\", file_path.display(), error)\n  192 |             })?;\n  193 | \n  194 |             let issues = self.lint(&source_code, &file_path.to_string_lossy())?;\n  195 | \n  196 |             if !issues.is_empty() {\n  197 |                 has_issues = true;\n  198 |                 file_issues.insert(file_path.to_string_lossy().to_string(), issues);\n  199 |             }\n  200 |         }\n  201 | \n  202 |         // Print pretty output grouped by file and line\n  203 |         let mut file_iter = file_issues.iter().peekable();","lines":18}]},{"id":"LOGIC_2","type":"logic_error","type_label":"Logic Error","severity":"high","easy":false,"description":"`parse_ignore_comment` in ignore_patterns.rs searches patterns in order\n[\"gdlint-ignore-next-line\", \"gdlint-ignore-line\", \"gdlint-ignore\"], but the check in\n`parse_ignore_patterns` first tests `comment.contains(\"gdlint-ignore-next-line\")`, then\n`comment.contains(\"gdlint-ignore-line\") || comment.contains(\"gdlint-ignore\")`. The\n`parse_ignore_comment` function returns `Some(rules)` on the first pattern it finds in\nthe comment text, which is correct. However `parse_ignore_patterns` re-checks the raw\ncomment string to decide whether to apply to the current or next line. Because\n\"gdlint-ignore-next-line\" contains \"gdlint-ignore\" as a substring, the `else if`\nbranch for \"gdlint-ignore\" can never fire for a \"gdlint-ignore-next-line\" comment\n(the outer `if` eats it). But critically: a comment containing \"gdlint-ignore-line\"\nalso contains \"gdlint-ignore\" as a substring, so the order of checks matters. The\ncurrent order is correct (more-specific check first), but the dual-parse approach\nis fragile - if someone reorders the conditions the behavior silently breaks.","fix":"Parse the directive kind once (in `parse_ignore_comment` or a shared helper returning\nan enum) and return it alongside the rule set. Eliminates the duplicate substring search\nand the fragile ordering dependency.","locations":[{"ref":"src/linter/ignore_patterns.rs:26-34","code":"   26 |                 if comment.contains(\"gdlint-ignore-next-line\") {\n   27 |                     // Apply to the next line\n   28 |                     let target_line = line_number + 1;\n   29 |                     ignore_map.entry(target_line).or_default().extend(rules);\n   30 |                 } else if comment.contains(\"gdlint-ignore-line\")\n   31 |                     || comment.contains(\"gdlint-ignore\")\n   32 |                 {\n   33 |                     // Apply to the current line\n   34 |                     ignore_map.entry(line_number).or_default().extend(rules);","lines":9},{"ref":"src/linter/ignore_patterns.rs:46-50","code":"   46 |     let patterns = [\n   47 |         \"gdlint-ignore-next-line\",\n   48 |         \"gdlint-ignore-line\",\n   49 |         \"gdlint-ignore\",\n   50 |     ];","lines":5}]},{"id":"LOGIC_3","type":"logic_error","type_label":"Logic Error","severity":"high","easy":true,"description":"VariableNameRule emits the rule name \"load-variable-name\" (src/linter/rules/variable_name.rs:57) but that name is not registered in ALL_RULES. This means: (1) validate_rule_names rejects --disable load-variable-name as an invalid rule, so users cannot disable the check through the CLI, and (2) get_all_rule_names() / --list-rules never shows it. The rule exists in practice but is invisible to the rule management system.","fix":"Either add a separate RuleDefinition entry for \"load-variable-name\" in ALL_RULES, or merge it into the existing \"variable-name\" entry so it can be disabled through normal means.","locations":[{"ref":"src/linter/rules/variable_name.rs:57","code":"   57 |                         \"load-variable-name\".to_string(),","lines":1},{"ref":"src/linter/rules.rs:77-150","code":"   77 | pub const ALL_RULES: &[RuleDefinition] = &[\n   78 |     RuleDefinition {\n   79 |         name: \"duplicated-load\",\n   80 |         create: |_config| {\n   81 |             Box::new(DuplicatedLoadRule {\n   82 |                 load_paths: std::collections::HashMap::new(),\n   83 |             })\n   84 |         },\n   85 |     },\n   86 |     RuleDefinition {\n   87 |         name: \"standalone-expression\",\n   88 |         create: |_config| Box::new(StandaloneExpressionRule),\n   89 |     },\n   90 |     RuleDefinition {\n   91 |         name: \"unnecessary-pass\",\n   92 |         create: |_config| Box::new(UnnecessaryPassRule),\n   93 |     },\n   94 |     RuleDefinition {\n   95 |         name: \"unused-argument\",\n   96 |         create: |_config| Box::new(UnusedArgumentRule),\n   97 |     },\n   98 |     RuleDefinition {\n   99 |         name: \"comparison-with-itself\",\n  100 |         create: |_config| Box::new(ComparisonWithItselfRule),\n  101 |     },\n  102 |     RuleDefinition {\n  103 |         name: \"private-access\",\n  104 |         create: |_config| Box::new(PrivateAccessRule),\n  105 |     },\n  106 |     RuleDefinition {\n  107 |         name: \"max-line-length\",\n  108 |         create: |config| Box::new(MaxLineLengthRule::new(config)),\n  109 |     },\n  110 |     RuleDefinition {\n  111 |         name: \"no-else-return\",\n  112 |         create: |_config| Box::new(NoElseReturnRule),\n  113 |     },\n  114 |     RuleDefinition {\n  115 |         name: \"function-name\",\n  116 |         create: |_config| Box::new(FunctionNameRule),\n  117 |     },\n  118 |     RuleDefinition {\n  119 |         name: \"class-name\",\n  120 |         create: |_config| Box::new(ClassNameRule),\n  121 |     },\n  122 |     RuleDefinition {\n  123 |         name: \"signal-name\",\n  124 |         create: |_config| Box::new(SignalNameRule),\n  125 |     },\n  126 |     RuleDefinition {\n  127 |         name: \"variable-name\",\n  128 |         create: |_config| Box::new(VariableNameRule),\n  129 |     },\n  130 |     RuleDefinition {\n  131 |         name: \"function-argument-name\",\n  132 |         create: |_config| Box::new(FunctionArgumentNameRule),\n  133 |     },\n  134 |     RuleDefinition {\n  135 |         name: \"loop-variable-name\",\n  136 |         create: |_config| Box::new(LoopVariableNameRule),\n  137 |     },\n  138 |     RuleDefinition {\n  139 |         name: \"enum-name\",\n  140 |         create: |_config| Box::new(EnumNameRule),\n  141 |     },\n  142 |     RuleDefinition {\n  143 |         name: \"enum-member-name\",\n  144 |         create: |_config| Box::new(EnumMemberNameRule),\n  145 |     },\n  146 |     RuleDefinition {\n  147 |         name: \"constant-name\",\n  148 |         create: |_config| Box::new(ConstantNameRule),\n  149 |     },\n  150 | ];","lines":74}]},{"id":"LOGIC_4","type":"logic_error","type_label":"Logic Error","severity":"high","easy":true,"description":"`extract_variable_name` strips the prefix `\"var \"` from the node text. For static variables the node\ntext begins with `\"static var \"`, so `strip_prefix(\"var \")` fails and the function returns `\"unknown_var\"`.\nEvery static variable therefore gets the name `\"unknown_var\"` in its `GDScriptTokenKind::StaticVariable`\nvariant, making all static variables appear identical to the sort and to the safe-mode signature check.","fix":"Strip the `\"static \"` prefix before attempting the `\"var \"` strip:\n\n    let text = text.trim_start_matches(\"static \").trim_start();\n    let Some(name) = text.strip_prefix(\"var \") else {\n        return Ok(\"unknown_var\".to_string());\n    };","locations":[{"ref":"src/reorder.rs:780-792","code":"  780 | fn extract_variable_name(node: Node, content: &str) -> Result<String, Box<dyn std::error::Error>> {\n  781 |     let text = node.utf8_text(content.as_bytes())?;\n  782 | \n  783 |     let Some(name) = text.strip_prefix(\"var \") else {\n  784 |         return Ok(\"unknown_var\".to_string());\n  785 |     };\n  786 | \n  787 |     if let Some((name, _)) = name.split_once(|c: char| c == ':' || c == '=' || c.is_whitespace()) {\n  788 |         return Ok(name.trim().to_string());\n  789 |     }\n  790 | \n  791 |     Ok(name.trim().to_string())\n  792 | }","lines":13}]},{"id":"LOGIC_5","type":"logic_error","type_label":"Logic Error","severity":"high","easy":true,"description":"`extract_function_name` strips the prefix `\"func \"` from the node text. For static functions the\ntree-sitter node text begins with `\"static func \"`, so `strip_prefix(\"func \")` fails and the function\nreturns `\"unknown_func\"`. All static functions end up with the name `\"unknown_func\"` in their\n`GDScriptTokenKind::Method` variant. This breaks deterministic reordering (all static functions compare\nequal by name) and, in safe mode, corrupts the signature comparison so any file with multiple static\nfunctions will always fail the safe-mode check.","fix":"Strip the `\"static \"` prefix first, then strip `\"func \"`:\n\n    let text = text.trim_start_matches(\"static \").trim_start();\n    let Some(name) = text.strip_prefix(\"func \") else {\n        return Ok(\"unknown_func\".to_string());\n    };","locations":[{"ref":"src/reorder.rs:795-806","code":"  795 | fn extract_function_name(node: Node, content: &str) -> Result<String, Box<dyn std::error::Error>> {\n  796 |     let text = node.utf8_text(content.as_bytes())?;\n  797 | \n  798 |     let Some(name) = text.strip_prefix(\"func \") else {\n  799 |         return Ok(\"unknown_func\".to_string());\n  800 |     };\n  801 | \n  802 |     if let Some((name, _)) = name.split_once('(') {\n  803 |         Ok(name.trim().to_string())\n  804 |     } else {\n  805 |         Ok(\"unknown_func\".to_string())\n  806 |     }","lines":12}]},{"id":"LOGIC_6","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"The `regex_replace_all_outside_strings_or_comments` function only checks whether the\n*start byte* of a regex match falls inside a string or comment node.  If a regex match\nstarts outside a string but extends into one (e.g. the dangling-comma pattern\n`([^\\n\\r])\\n\\s+,` matching text immediately before a string literal on the same line),\nthe string's content can be modified.  The node lookup is:\n    `descendant_for_byte_range(start_byte, start_byte)` (a zero-length range).\nThis finds the node that contains only the start byte, missing cases where the match\nspans a boundary between non-string and string.","fix":"Check the node for *both* the start byte and end byte of the match, and skip the\nreplacement if either endpoint falls inside a string or comment:\n    let end_node = self.tree.root_node()\n        .descendant_for_byte_range(old_end_byte - 1, old_end_byte - 1)\n        .unwrap();\n    if is_string_or_comment(node) || is_string_or_comment(end_node) { continue; }","locations":[{"ref":"src/formatter.rs:531-545","code":"  531 |             let node = self\n  532 |                 .tree\n  533 |                 .root_node()\n  534 |                 .descendant_for_byte_range(start_byte, start_byte)\n  535 |                 .unwrap();\n  536 |             // String nodes may also contain escape_sequence nodes.  These are\n  537 |             // found when a backslash is present within a string.\n  538 |             // Comment nodes contain docstrings (##) and regular comments (#).\n  539 |             // We skip all of these to avoid modifying content inside strings or comments.\n  540 |             if node.kind() == \"string\"\n  541 |                 || node.kind() == \"escape_sequence\"\n  542 |                 || node.kind() == \"comment\"\n  543 |             {\n  544 |                 continue;\n  545 |             }","lines":15}]},{"id":"LOGIC_7","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`regex_replace_all_outside_strings_or_comments` checks `node.kind() == \"string\"`, `\"escape_sequence\"`, and `\"comment\"` to skip replacements inside string literals and comments (lines 540-543). It does NOT check for `\"string_name\"`, which is GDScript's `&\"...\"` string-name syntax. This means regex replacements (e.g. `fix_trailing_spaces`, `fix_dangling_semicolons`) could modify the content of `string_name` literals. `string_name` is a separate named node type in the tree-sitter-gdscript grammar with the same structure as `string`.","fix":"Add `|| node.kind() == \"string_name\"` to the guard condition on line 540.","locations":[{"ref":"src/formatter.rs:540-544","code":"  540 |             if node.kind() == \"string\"\n  541 |                 || node.kind() == \"escape_sequence\"\n  542 |                 || node.kind() == \"comment\"\n  543 |             {\n  544 |                 continue;","lines":5}]},{"id":"LOGIC_8","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`regex_replace_all_outside_strings_or_comments` only checks whether a matched node is\nof kind `\"string\"`, `\"escape_sequence\"`, or `\"comment\"`, but does NOT walk up the\nancestor chain.  A token whose kind is, say, `\"identifier\"` that lives *inside* a\nmultiline string literal will have the ancestor `\"string\"` but its own kind will be\nsomething else.  As a result, regex replacements can silently corrupt the content of\nmultiline strings.","fix":"After finding the leaf node, walk its ancestors:\n    fn is_inside_string_or_comment(mut node: tree_sitter::Node) -> bool {\n        loop {\n            match node.kind() {\n                \"string\" | \"escape_sequence\" | \"comment\" => return true,\n                _ => {}\n            }\n            match node.parent() {\n                Some(p) => node = p,\n                None => return false,\n            }\n        }\n    }\nUse `is_inside_string_or_comment(node)` as the skip condition.","locations":[{"ref":"src/formatter.rs:540-545","code":"  540 |             if node.kind() == \"string\"\n  541 |                 || node.kind() == \"escape_sequence\"\n  542 |                 || node.kind() == \"comment\"\n  543 |             {\n  544 |                 continue;\n  545 |             }","lines":6}]},{"id":"LOGIC_9","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`extract_inline_extends` uses `original_text.find(\"extends\")` which performs a plain substring search.\nAny `class_name` token whose text contains the letters \"extends\" inside a word -- for example\n`class_name ExtendsManager` or `class_name RendererExtensions` -- would incorrectly match, returning a\nspurious `\"Extends::ExtendsManager\"` (or similar) signature. In safe mode this creates a phantom entry\nin the signature diff and causes `ensure_top_level_tokens_match` to falsely report a mismatch after\nevery formatting pass on such a file.","fix":"Match the `extends` keyword with a word boundary instead. A simple approach is to parse the text\nfor `\" extends \"` (with surrounding whitespace) or use a regex. Alternatively, use the tree-sitter\nnode directly: traverse the children of the `class_name_statement` node to find an `extends_statement`\nchild, which is how the AST already encodes inline extends.","locations":[{"ref":"src/formatter.rs:857-875","code":"  857 | fn inline_extends_signature(token_kind: &GDScriptTokenKind, original_text: &str) -> Option<String> {\n  858 |     match token_kind {\n  859 |         GDScriptTokenKind::ClassName(_) => {\n  860 |             let extends_part = extract_inline_extends(original_text)?;\n  861 |             Some(format!(\"Extends::{extends_part}\"))\n  862 |         }\n  863 |         _ => None,\n  864 |     }\n  865 | }\n  866 | \n  867 | fn extract_inline_extends(original_text: &str) -> Option<String> {\n  868 |     let extends_index = original_text.find(\"extends\")?;\n  869 |     let extends_slice = &original_text[extends_index..];\n  870 |     let trimmed = extends_slice.trim();\n  871 |     if trimmed.is_empty() {\n  872 |         None\n  873 |     } else {\n  874 |         Some(trimmed.to_string())\n  875 |     }","lines":19}]},{"id":"LOGIC_10","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`extract_inline_extends` (line 867-876) returns `Some(trimmed)` where `trimmed` is\n`extends_slice.trim()`.  `extends_slice` starts at the word \"extends\" (not after it),\nso `trimmed` will always start with the string \"extends\" itself - the function never\nextracts just the parent class name.  As a result, the generated signature key is\n`\"Extends::extends SomeClass\"` instead of `\"Extends::SomeClass\"`, which happens to\nbe consistent between original and formatted code (both calls go through the same\nfunction), so safe-mode comparison still passes.  However the key is misleading and\nany future change that extracts the name differently from only one side will silently\nbreak safe-mode detection.","fix":"Slice past the \"extends\" keyword and any following whitespace:\n    let rest = &original_text[extends_index + \"extends\".len()..];\n    let trimmed = rest.trim();\n    if trimmed.is_empty() { return None; }\n    Some(trimmed.to_string())","locations":[{"ref":"src/formatter.rs:867-876","code":"  867 | fn extract_inline_extends(original_text: &str) -> Option<String> {\n  868 |     let extends_index = original_text.find(\"extends\")?;\n  869 |     let extends_slice = &original_text[extends_index..];\n  870 |     let trimmed = extends_slice.trim();\n  871 |     if trimmed.is_empty() {\n  872 |         None\n  873 |     } else {\n  874 |         Some(trimmed.to_string())\n  875 |     }\n  876 | }","lines":10}]},{"id":"LOGIC_11","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`lint_files_pretty` collects file paths into a `HashMap<String, Vec<LintIssue>>` and then iterates\nit with `.iter()`. HashMap iteration order is not guaranteed in Rust (it is randomized per-process).\nThis means the pretty-formatted output lists files in a different, non-deterministic order on every\nrun, which makes automated diffing of lint output unreliable and confuses users comparing two runs.\nThe standard `lint_files_standard` mode processes files in the order they were discovered (already\nsorted), but the pretty mode silently drops that ordering.","fix":"Use a `BTreeMap<String, Vec<LintIssue>>` instead of `HashMap`, or sort the keys before printing:\n\n    let mut file_keys: Vec<&String> = file_issues.keys().collect();\n    file_keys.sort();\n    for file_path in file_keys { ... }","locations":[{"ref":"src/linter.rs:180-253","code":"  180 |     fn lint_files_pretty(\n  181 |         &mut self,\n  182 |         gdscript_files: &[PathBuf],\n  183 |         with_colors: bool,\n  184 |     ) -> Result<bool, Box<dyn std::error::Error>> {\n  185 |         use std::collections::HashMap;\n  186 |         let mut file_issues: HashMap<String, Vec<_>> = HashMap::new();\n  187 |         let mut has_issues = false;\n  188 | \n  189 |         for file_path in gdscript_files {\n  190 |             let source_code = fs::read_to_string(file_path).map_err(|error| {\n  191 |                 format!(\"Failed to read file {}: {}\", file_path.display(), error)\n  192 |             })?;\n  193 | \n  194 |             let issues = self.lint(&source_code, &file_path.to_string_lossy())?;\n  195 | \n  196 |             if !issues.is_empty() {\n  197 |                 has_issues = true;\n  198 |                 file_issues.insert(file_path.to_string_lossy().to_string(), issues);\n  199 |             }\n  200 |         }\n  201 | \n  202 |         // Print pretty output grouped by file and line\n  203 |         let mut file_iter = file_issues.iter().peekable();\n  204 |         while let Some((file_path, issues)) = file_iter.next() {\n  205 |             let bold = if with_colors { \"\\x1b[1m\" } else { \"\" };\n  206 |             let reset = if with_colors { \"\\x1b[0m\" } else { \"\" };\n  207 | \n  208 |             println!(\"{}{}{}\", bold, file_path, reset);\n  209 | \n  210 |             // Group issues by line number\n  211 |             let mut line_issues: HashMap<usize, Vec<_>> = HashMap::new();\n  212 |             for issue in issues {\n  213 |                 line_issues.entry(issue.line).or_default().push(issue);\n  214 |             }\n  215 | \n  216 |             // Sort by line number and print\n  217 |             let mut sorted_lines: Vec<_> = line_issues.keys().collect();\n  218 |             sorted_lines.sort();\n  219 | \n  220 |             for (i, &line_num) in sorted_lines.iter().enumerate() {\n  221 |                 if let Some(line_issues) = line_issues.get(line_num) {\n  222 |                     println!(\"    {}:{}\", file_path, line_num);\n  223 |                     for issue in line_issues {\n  224 |                         let (severity_str, severity_color) = match issue.severity {\n  225 |                             LintSeverity::Error => (\"ERROR\", \"\\x1b[31m\"),  // Red\n  226 |                             LintSeverity::Warning => (\"WARN\", \"\\x1b[33m\"), // Yellow\n  227 |                         };\n  228 | \n  229 |                         if with_colors {\n  230 |                             println!(\n  231 |                                 \"        {}{}\\x1b[0m: `{}`\",\n  232 |                                 severity_color, severity_str, issue.rule\n  233 |                             );\n  234 |                         } else {\n  235 |                             println!(\"        {}: `{}`\", severity_str, issue.rule);\n  236 |                         }\n  237 |                         println!(\"        {}\", issue.message);\n  238 |                     }\n  239 | \n  240 |                     // Add newline between line groups (except for the last line group)\n  241 |                     if i < sorted_lines.len() - 1 {\n  242 |                         println!();\n  243 |                     }\n  244 |                 }\n  245 |             }\n  246 | \n  247 |             // Add separator between files (except for the last file)\n  248 |             if file_iter.peek().is_some() {\n  249 |                 println!(\"\\n{}\", \"-\".repeat(60));\n  250 |                 println!();\n  251 |             }\n  252 |         }\n  253 | ","lines":74}]},{"id":"LOGIC_12","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`parse_ignore_comment` returns `Some(HashSet::new())` (empty set) when no rule names\nfollow the directive keyword. This empty set is documented as \"ignore all\". However,\n`should_ignore_rule` checks `ignored_rules.is_empty()` to mean \"ignore all\". This\nsemantic is invisible to callers: an empty HashSet from a normal HashSet::new() is\nindistinguishable from the \"ignore all\" sentinel. If any code path accidentally inserts\nan empty entry into the map (e.g., via `or_default()` without populating it), all rules\non that line would be silently suppressed. The invariant is not enforced or documented\nat the type level.","fix":"Use an explicit enum: `enum IgnoreSpec { All, Rules(HashSet<String>) }` so the\n\"ignore all\" case is unambiguous and cannot arise from an accidentally empty set.","locations":[{"ref":"src/linter/ignore_patterns.rs:61-63","code":"   61 |                 // No specific rules mentioned, ignore all rules\n   62 |                 return Some(HashSet::new()); // Empty set means ignore all\n   63 |             }","lines":3},{"ref":"src/linter/ignore_patterns.rs:87-88","code":"   87 |         // If the set is empty, it means ignore all rules\n   88 |         ignored_rules.is_empty() || ignored_rules.contains(rule_name)","lines":2}]},{"id":"LOGIC_13","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":true,"description":"constant_name.rs and variable_name.rs treat load() and preload() inconsistently. VariableNameRule.is_load_call() accepts both \"load\" and \"preload\", so variables assigned from either get relaxed PascalCase/snake_case validation. But ConstantNameRule.is_preload_call() only accepts \"preload\". A constant like `const X = load(\"res://scene.tscn\")` is treated as a plain constant (requires CONSTANT_CASE) rather than a load constant (allows PascalCase), which is wrong. The inline comment on line 52 even says 'For all load/preload constants' but the code only handles preload.","fix":"Change is_preload_call in constant_name.rs to also match \"load\", mirroring variable_name.rs: `function_name == \"preload\" || function_name == \"load\"`. Update the error message accordingly.","locations":[{"ref":"src/linter/rules/constant_name.rs:21-30","code":"   21 |     fn is_preload_call(&self, node: &Node, source_code: &str) -> bool {\n   22 |         if node.kind() == \"call\"\n   23 |             && let Some(function_node) = node.child(0)\n   24 |         {\n   25 |             let function_name = get_node_text(&function_node, source_code);\n   26 |             return function_name == \"preload\";\n   27 |         }\n   28 | \n   29 |         false\n   30 |     }","lines":10},{"ref":"src/linter/rules/constant_name.rs:52","code":"   52 |                 // For all load/preload constants, check load naming rules","lines":1},{"ref":"src/linter/rules/variable_name.rs:17-25","code":"   17 |     fn is_load_call(&self, node: &Node, source_code: &str) -> bool {\n   18 |         if node.kind() == \"call\"\n   19 |             && let Some(function_node) = node.child(0)\n   20 |         {\n   21 |             let function_name = get_node_text(&function_node, source_code);\n   22 |             return function_name == \"load\" || function_name == \"preload\";\n   23 |         }\n   24 |         false\n   25 |     }","lines":9}]},{"id":"LOGIC_14","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`no_else_return.rs`: The `all_branches_return` flag is only set to `false` when an\n`elif` body does NOT end with a return. But if an `elif` body DOES end with a return,\n`all_branches_return` stays `true`. However, if the first `elif` does not end with\nreturn (`all_branches_return = false`), a subsequent `elif` that does end with return\ncannot set `all_branches_return` back to `true`. So the logic of checking whether all\nbranches return is correct in that direction.\n\nThe real bug: `all_branches_return` starts as `if_body_ends_with_return`. If the `if`\nbody does NOT end with return (`all_branches_return = false`), but a subsequent `elif`\nbody DOES end with return, the `else` after it is NOT flagged, even though all branches\nactually do return. The flag can only go from `true` to `false`, never the reverse.\nThis means the rule under-reports in some multi-elif chains. While conservative (no\nfalse positives), the rule silently misses real `else-after-return` cases.","fix":"Track per-elif whether it ends with return and accumulate the \"all branches return\"\nstatus correctly by checking each branch individually, not just flip to false.","locations":[{"ref":"src/linter/rules/no_else_return.rs:50","code":"   50 |         let mut all_branches_return = if_body_ends_with_return;","lines":1},{"ref":"src/linter/rules/no_else_return.rs:57-71","code":"   57 |                     if if_body_ends_with_return {\n   58 |                         let (line, column) = get_line_column(&child_node);\n   59 |                         issues.push(LintIssue::new(\n   60 |                             line,\n   61 |                             column,\n   62 |                             \"no-else-return\".to_string(),\n   63 |                             LintSeverity::Warning,\n   64 |                             \"Unnecessary 'elif' after 'if' block that ends with 'return'. Use 'if' instead\".to_string(),\n   65 |                         ));\n   66 |                     }\n   67 | \n   68 |                     if let Some(elif_body) = child_node.child_by_field_name(\"body\")\n   69 |                         && !self.body_ends_with_return(&elif_body, source_code)\n   70 |                     {\n   71 |                         all_branches_return = false;","lines":15}]},{"id":"LOGIC_15","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`unused_argument.rs`: `is_identifier_used_in_node` walks the entire function body AST\nlooking for any `identifier` node whose text matches the parameter name. This means if\na parameter is named `x` and the function body contains `var x = 5` (a local variable\nshadowing the parameter), the rule will report the argument as used even though the\nouter `x` (the parameter) is never actually referenced. This produces false negatives.\n\nConversely, if the parameter name matches a string literal content (unlikely via AST\nsince it checks node kind \"identifier\") this is fine. The core issue is shadowing.","fix":"Track scopes during traversal: when encountering a variable declaration that introduces\na new binding with the same name, stop searching deeper in that scope branch.","locations":[{"ref":"src/linter/rules/unused_argument.rs:12-45","code":"   12 |     fn is_identifier_used_in_node(&self, node: &Node, identifier: &str, source_code: &str) -> bool {\n   13 |         let mut cursor = node.walk();\n   14 | \n   15 |         fn check_usage(\n   16 |             cursor: &mut tree_sitter::TreeCursor,\n   17 |             identifier: &str,\n   18 |             source_code: &str,\n   19 |         ) -> bool {\n   20 |             let node = cursor.node();\n   21 | \n   22 |             if node.kind() == \"identifier\" {\n   23 |                 let node_text = get_node_text(&node, source_code);\n   24 |                 if node_text == identifier {\n   25 |                     return true;\n   26 |                 }\n   27 |             }\n   28 | \n   29 |             if cursor.goto_first_child() {\n   30 |                 loop {\n   31 |                     if check_usage(cursor, identifier, source_code) {\n   32 |                         return true;\n   33 |                     }\n   34 |                     if !cursor.goto_next_sibling() {\n   35 |                         break;\n   36 |                     }\n   37 |                 }\n   38 |                 cursor.goto_parent();\n   39 |             }\n   40 | \n   41 |             false\n   42 |         }\n   43 | \n   44 |         check_usage(&mut cursor, identifier, source_code)\n   45 |     }","lines":34}]},{"id":"LOGIC_16","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`get_token_kind` maps `GDScriptTokenKind::Unknown` to `TokenKind::Method` (line 225).\nBecause `Unknown` nodes have get_priority() == 255 (lowest priority, sorted last), they\nappear after inner classes in the output. When the spacing logic in `build_reordered_code`\nsees an `Unknown` token after an `InnerClass` (different group), it calls the `needs_spacing`\nbranch for a group transition. Since `is_function` is false and `is_inner_class` is false,\nthe else branch (line 947) emits only a single `\\n`. But the preceding inner class token\nalready ends with `\\n`, producing just one blank line - the same as between two variables,\nnot the expected two-newline separation that unknown/unsupported top-level syntax deserves.\nAdditionally, an Unknown token followed by another Unknown token would have `previous_kind\n== current_token_type == TokenKind::Method`, causing the `is_function` check (line 923)\nto fire and emit `\\n\\n`, which is incorrect for non-function unknown nodes.","fix":"Add a `TokenKind::Unknown` variant to the `TokenKind` enum and map\n`GDScriptTokenKind::Unknown` to it. In `build_reordered_code`, handle `Unknown` explicitly\nso consecutive unknown nodes do not receive function-style double spacing.","locations":[{"ref":"src/reorder.rs:225","code":"  225 |         GDScriptTokenKind::Unknown(_) => TokenKind::Method,","lines":1},{"ref":"src/reorder.rs:907-937","code":"  907 |         let current_token_type = get_token_kind(&current_token.token_kind);\n  908 |         let is_function = matches!(current_token.token_kind, GDScriptTokenKind::Method(_, _, _));\n  909 | \n  910 |         let is_inner_class = matches!(\n  911 |             current_token.token_kind,\n  912 |             GDScriptTokenKind::InnerClass(_, _)\n  913 |         );\n  914 |         // If true, we need to add spacing before this element, either single or\n  915 |         // double line breaks depending on the context.\n  916 |         let needs_spacing = if output.is_empty() {\n  917 |             false\n  918 |         } else if let Some(previous_kind) = previous_token_kind {\n  919 |             if previous_kind != current_token_type {\n  920 |                 // We're leaving one group of tokens for another (like previous\n  921 |                 // was variables, now we're seeing a function) -> needs spacing\n  922 |                 true\n  923 |             } else if is_function {\n  924 |                 // Between functions we always want two line breaks\n  925 |                 true\n  926 |             } else if is_inner_class && previous_kind == TokenKind::InnerClass {\n  927 |                 // Between inner classes, same as functions\n  928 |                 true\n  929 |             } else {\n  930 |                 // If we reach here we're seeing the same kind of token as\n  931 |                 // before, like two regular variables in a row or two signals in\n  932 |                 // a row - we don't need extra spacing\n  933 |                 false\n  934 |             }\n  935 |         } else {\n  936 |             false\n  937 |         };","lines":31}]},{"id":"LOGIC_17","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":true,"description":"region_end_comment is a single Option<String> rather than a Vec, so if multiple #endregion nodes appear without a matching function (neither a preceding Method element), the Option is overwritten at line 419 and only the last #endregion survives. Earlier ones are silently dropped.","fix":"Change region_end_comment to Vec<String>, push instead of assigning, and iterate over the vec when attaching to the last element at lines 569-581.","locations":[{"ref":"src/reorder.rs:350","code":"  350 |     let mut region_end_comment = None;","lines":1},{"ref":"src/reorder.rs:416-420","code":"  416 |                 if !attached {\n  417 |                     // We didn't find a function to attach to, so we save this\n  418 |                     // to handle down below\n  419 |                     region_end_comment = Some(text.clone());\n  420 |                 }","lines":5}]},{"id":"LOGIC_18","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":true,"description":"Class docstrings are silently dropped when a file has a class_name statement but no extends statement and no other declarations follow. The class_name_statement branch (line 449) intentionally skips emitting the Docstring element, saving it for extends. But if no extends_statement and no subsequent declaration exists (e.g. a file with only 'class_name Foo' and ## comments), class_docstring_attached stays false and class_docstring_comments is never emitted anywhere.","fix":"After the main loop, if !class_docstring_attached and !class_docstring_comments.is_empty(), emit the Docstring element (the same way the _ branch does it at lines 517-527).","locations":[{"ref":"src/reorder.rs:449-476","code":"  449 |             \"class_name_statement\" => {\n  450 |                 if let Some(element) = reorderable_element {\n  451 |                     // Don't attach class docstring to class_name, save it for extends\n  452 |                     let mut attachments: Vec<&PendingAttachment> =\n  453 |                         pending_annotations.iter().collect();\n  454 |                     attachments.extend(pending_comments.iter());\n  455 |                     attachments.sort_by_key(|attachment| attachment.start_byte);\n  456 |                     let non_docstring_comments: Vec<String> = attachments\n  457 |                         .into_iter()\n  458 |                         .filter_map(|attachment| {\n  459 |                             if !class_docstring_comments.contains(&attachment.text) {\n  460 |                                 Some(attachment.text.clone())\n  461 |                             } else {\n  462 |                                 None\n  463 |                             }\n  464 |                         })\n  465 |                         .collect();\n  466 |                     elements.push(GDScriptTokensWithComments {\n  467 |                         token_kind: element,\n  468 |                         attached_comments: non_docstring_comments,\n  469 |                         trailing_comments: Vec::new(),\n  470 |                         original_text: text,\n  471 |                         start_byte: node.start_byte(),\n  472 |                         end_byte: node.end_byte(),\n  473 |                     });\n  474 |                     pending_comments.clear();\n  475 |                     pending_annotations.clear();\n  476 |                 }","lines":28},{"ref":"src/reorder.rs:513-527","code":"  513 |                     if !class_docstring_attached\n  514 |                         && !class_docstring_comments.is_empty()\n  515 |                         && !found_extends_declaration\n  516 |                     {\n  517 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  518 |                         elements.push(GDScriptTokensWithComments {\n  519 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  520 |                             attached_comments: Vec::new(),\n  521 |                             trailing_comments: Vec::new(),\n  522 |                             original_text: docstring_text,\n  523 |                             start_byte: 0,\n  524 |                             end_byte: 0,\n  525 |                         });\n  526 |                         class_docstring_attached = true;\n  527 |                     }","lines":15}]},{"id":"LOGIC_19","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":true,"description":"In `classify_element`, the check `text.contains(\"extends\")` on a `class_name_statement`\nnode is too broad. If a class name itself contains the lowercase substring \"extends\"\n(e.g., `class_name PreextendsHelper`), the node text `\"class_name PreextendsHelper\"` would\nmatch the condition and `splitn(2, \"extends\")` would truncate the stored name to `\"class_name Pre\"`.\nThe reconstructed `GDScriptTokenKind::ClassName` would carry a truncated name.\nWhile this does not corrupt the output text (original_text is used for reconstruction),\nit pollutes the internal classification with a wrong name that could affect future\nlogic relying on the stored name.","fix":"Use a word-boundary approach: check for \"extends \" (with trailing space) or check that\n\"extends\" is preceded by whitespace. For example:\n  if text.contains(\" extends \") || text.ends_with(\" extends\") {\nOr parse the node's children via the tree-sitter API instead of text matching.","locations":[{"ref":"src/reorder.rs:628-637","code":"  628 |             if text.contains(\"extends\") {\n  629 |                 let parts: Vec<&str> = text.splitn(2, \"extends\").collect();\n  630 |                 if parts.len() == 2 {\n  631 |                     // We'll handle this case in the extraction logic\n  632 |                     Ok(Some(GDScriptTokenKind::ClassName(\n  633 |                         parts[0].trim().to_string(),\n  634 |                     )))\n  635 |                 } else {\n  636 |                     Ok(Some(GDScriptTokenKind::ClassName(text.to_string())))\n  637 |                 }","lines":10}]},{"id":"LOGIC_20","type":"logic_error","type_label":"Logic Error","severity":"medium","easy":false,"description":"`classify_variable_statement` detects annotations with plain string searches\n(`text.contains(\"@export\")`, `text.contains(\"@onready\")`, `text.contains(\"static var\")`).\nThe `text` variable is the full source text of the `variable_statement` node, which includes the\nvariable's initializer. A variable whose default value is a string literal containing one of those\ntokens -- e.g. `var hint = \"@export var x = 1\"` -- would be misclassified as an export variable.\nSimilarly, `text.contains(\"static var\")` would misclassify a variable whose string value contains\nthat phrase.","fix":"Classify variables by inspecting the tree-sitter AST rather than the raw text. Walk the node's\nchildren to look for `annotation` child nodes with specific names, and check for the `static`\nkeyword as a named sibling or child, rather than searching the raw source string.","locations":[{"ref":"src/reorder.rs:696-731","code":"  696 | fn classify_variable_statement(\n  697 |     node: Node,\n  698 |     content: &str,\n  699 | ) -> Result<Option<GDScriptTokenKind>, Box<dyn std::error::Error>> {\n  700 |     let text = node.utf8_text(content.as_bytes())?;\n  701 |     let variable_name = extract_variable_name(node, content)?;\n  702 |     let is_private = variable_name.starts_with('_');\n  703 | \n  704 |     // Look for annotations in the node's text string, which we use to sort the\n  705 |     // variables\n  706 |     let has_export = text.contains(\"@export\");\n  707 |     let has_onready = text.contains(\"@onready\");\n  708 |     let has_static = text.contains(\"static var\");\n  709 | \n  710 |     if has_export {\n  711 |         Ok(Some(GDScriptTokenKind::ExportVariable(\n  712 |             variable_name,\n  713 |             is_private,\n  714 |         )))\n  715 |     } else if has_onready {\n  716 |         Ok(Some(GDScriptTokenKind::OnReadyVariable(\n  717 |             variable_name,\n  718 |             is_private,\n  719 |         )))\n  720 |     } else if has_static {\n  721 |         Ok(Some(GDScriptTokenKind::StaticVariable(\n  722 |             variable_name,\n  723 |             is_private,\n  724 |         )))\n  725 |     } else {\n  726 |         Ok(Some(GDScriptTokenKind::RegularVariable(\n  727 |             variable_name,\n  728 |             is_private,\n  729 |         )))\n  730 |     }\n  731 | }","lines":36}]},{"id":"LOGIC_21","type":"logic_error","type_label":"Logic Error","severity":"low","easy":true,"description":"The benchmark only warms up the short-file path (10 iterations, line 29-31) before measuring short-file performance, but there is no warmup for the long-file path. The first ITERATIONS runs of the long file include cold OS page-cache and any one-time initialisation costs, making long-file timings slightly less accurate than short-file timings.","fix":"Add a warmup loop for the long file (e.g. 5 iterations) immediately before the long-file timing block.","locations":[{"ref":"src/bin/benchmark.rs:28-47","code":"   28 |     println!(\"Running short file warmup (10 iterations)\");\n   29 |     for _ in 0..10 {\n   30 |         let _ = format_gdscript_with_config(&short_content, &config)?;\n   31 |     }\n   32 | \n   33 |     println!(\"Benchmarking short file ({} iterations)\", ITERATIONS);\n   34 |     let mut start = Instant::now();\n   35 |     for _ in 0..ITERATIONS {\n   36 |         let _ = format_gdscript_with_config(&short_content, &config)?;\n   37 |     }\n   38 |     let duration_short_file = start.elapsed();\n   39 | \n   40 |     // Benchmark long file\n   41 |     println!(\"Benchmarking long file ({} iterations)...\", ITERATIONS);\n   42 |     start = Instant::now();\n   43 |     for _ in 0..ITERATIONS {\n   44 |         let _ = format_gdscript_with_config(&long_content, &config)?;\n   45 |     }\n   46 |     let long_time = start.elapsed();\n   47 | ","lines":20}]},{"id":"LOGIC_22","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"In make_release.rs, the Cargo.toml version replacement uses a plain string replace on the full file text (line 141-143). It replaces every occurrence of `version = \"<current_version>\"` including any dependency entry that happens to share the same version string as the package itself. While unlikely with the current pinned dependencies, any future coincidental version match would silently corrupt the dependency entry.","fix":"Replace only the first occurrence, or use a regex anchored to the `[package]` section, e.g. replace only a line that starts with `^version = ` at the top of the file.","locations":[{"ref":"src/bin/make_release.rs:141-144","code":"  141 |     let updated_cargo_toml = cargo_toml.replace(\n  142 |         &format!(\"version = \\\"{}\\\"\", current_version),\n  143 |         &format!(\"version = \\\"{}\\\"\", new_version),\n  144 |     );","lines":4}]},{"id":"LOGIC_23","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"The `add_newlines_after_extends_statement` regex (line 226) only matches extends names matching `[a-zA-Z0-9]+` or `\".*?\"`. GDScript 4 allows dotted type names in extends (e.g. `extends Node.SubType`), which contain `.` and are therefore not matched by `[a-zA-Z0-9]+`. When a file uses dotted extends, the post-formatting blank line insertion silently does not run, leaving the formatting potentially incomplete (no blank line after the extends statement).","fix":"Expand the `extends_name` capture group to `([a-zA-Z0-9_.]+|\".*?\")` to match dotted names.","locations":[{"ref":"src/formatter.rs:225-230","code":"  225 |         let re = RegexBuilder::new(\n  226 |             r#\"(?P<extends_line>^extends )(?P<extends_name>([a-zA-Z0-9]+|\".*?\"))\\n+((?P<doc>(?:^##.*\\n)+)(?:\\z|\\n))?\\n*(?P<EOF>\\z)?\"#,\n  227 |         )\n  228 |         .multi_line(true)\n  229 |         .build()\n  230 |         .expect(\"regex should compile\");","lines":6}]},{"id":"LOGIC_24","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"`extract_inline_extends` (line 868) uses a naive `original_text.find(\"extends\")` to locate the extends clause inside a `class_name_statement`. This substring search can match `\"extends\"` that appears inside an identifier (e.g. a class named `class_name ExtendsHelper`) or inside a trailing comment (`class_name Foo # extends something`). A false match produces a spurious `Extends::extends ...` signature entry in the safe-mode comparison, which could cause safe mode to incorrectly reject a valid formatting.","fix":"Use a word-boundary-aware search (e.g. a regex or a check that `extends` is preceded and followed by whitespace/end-of-string) rather than plain substring search.","locations":[{"ref":"src/formatter.rs:867-875","code":"  867 | fn extract_inline_extends(original_text: &str) -> Option<String> {\n  868 |     let extends_index = original_text.find(\"extends\")?;\n  869 |     let extends_slice = &original_text[extends_index..];\n  870 |     let trimmed = extends_slice.trim();\n  871 |     if trimmed.is_empty() {\n  872 |         None\n  873 |     } else {\n  874 |         Some(trimmed.to_string())\n  875 |     }","lines":9}]},{"id":"LOGIC_25","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"In `move_annotations`, the `annotations_to_move` iterator uses `map_while` (lines 1011-1029), which stops at the first element that does not match. This means if annotations appear in the order `@export @other_annotation @onready var x`, the backward scan finds `@onready` (moves it), then hits `@other_annotation` which is neither `onready` nor `export`, stops, and never collects `@export`. Only the trailing consecutive run of onready/export annotations is moved; any export annotation preceding a different annotation type is silently left behind, producing a structurally different tree that may cause `validate_formatting` to fail.","fix":"Use `filter_map` instead of `map_while` over the reversed range to collect ALL onready/export annotations regardless of interspersed other annotations, or scan the full range and collect non-consecutively.","locations":[{"ref":"src/formatter.rs:1009-1029","code":" 1009 |                     let annotations_to_move = (0..index)\n 1010 |                         .rev()\n 1011 |                         .map_while(|i| {\n 1012 |                             let child_id = self.nodes[parent_id].children[i];\n 1013 |                             let child = &self.nodes[child_id];\n 1014 |                             if child.grammar_name != \"annotation\" {\n 1015 |                                 return None;\n 1016 |                             }\n 1017 |                             let Some(annotation_name) = &self.nodes[child.children[0]].text else {\n 1018 |                                 return None;\n 1019 |                             };\n 1020 |                             if annotation_name != \"onready\" && annotation_name != \"export\" {\n 1021 |                                 return None;\n 1022 |                             }\n 1023 |                             let parent = &mut self.nodes[parent_id];\n 1024 |                             // When we found one, we remove it from the parent and collect them in a vector\n 1025 |                             let annotation_id = parent.children.remove(i);\n 1026 |                             index -= 1;\n 1027 |                             Some(annotation_id)\n 1028 |                         })\n 1029 |                         .collect::<Vec<_>>();","lines":21}]},{"id":"LOGIC_26","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"`calculate_end_position` (lines 1187-1197) counts bytes rather than UTF-8 characters\nfor `column`.  Multi-byte UTF-8 sequences (e.g. emoji in comments or strings) cause\n`column` to be counted in bytes while tree-sitter uses character counts for column\npositions.  The result is a mismatch between the `InputEdit` column value and\ntree-sitter's internal column representation, which can corrupt incremental re-parse\nafter regex replacements on files containing non-ASCII characters.","fix":"Use `str::chars()` or the Unicode scalar value count instead of iterating over bytes:\n\n    fn calculate_end_position(mut start: Point, slice: &str) -> Point {\n        for ch in slice.chars() {\n            if ch == '\\n' {\n                start.row += 1;\n                start.column = 0;\n            } else {\n                start.column += ch.len_utf8();\n            }\n        }\n        start\n    }\n\nNote: tree-sitter's column is byte-based in some versions; verify the exact semantics\nof the tree-sitter version in use before changing.","locations":[{"ref":"src/formatter.rs:1187-1197","code":" 1187 | fn calculate_end_position(mut start: Point, slice: &str) -> Point {\n 1188 |     for b in slice.as_bytes() {\n 1189 |         if *b == b'\\n' {\n 1190 |             start.row += 1;\n 1191 |             start.column = 0;\n 1192 |         } else {\n 1193 |             start.column += 1;\n 1194 |         }\n 1195 |     }\n 1196 |     start\n 1197 | }","lines":11}]},{"id":"LOGIC_27","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"`LintIssue::format` builds the output string with the format pattern\n`\"{}:{}:{}:{}: {}\"` using `(file_path, self.line, self.rule, severity_str, self.message)`.\nThe `self.column` field -- which the struct stores and the `LintIssue::new` constructor accepts --\nis silently omitted from the formatted output. Tools and editors that parse standard lint output\n(e.g., `file:line:col:severity:message`) will be unable to navigate to the exact column.","fix":"Include `self.column` in the format string:\n\n    format!(\n        \"{}:{}:{}:{}:{}: {}\",\n        file_path, self.line, self.column, self.rule, severity_str, self.message\n    )","locations":[{"ref":"src/linter.rs:50-59","code":"   50 |     pub fn format(&self, file_path: &str) -> String {\n   51 |         let severity_str = match self.severity {\n   52 |             LintSeverity::Error => \"error\",\n   53 |             LintSeverity::Warning => \"warning\",\n   54 |         };\n   55 |         format!(\n   56 |             \"{}:{}:{}:{}: {}\",\n   57 |             file_path, self.line, self.rule, severity_str, self.message\n   58 |         )\n   59 |     }","lines":10}]},{"id":"LOGIC_28","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"In ignore_patterns.rs, the `#` character is searched for using plain string search (line.find('#')). If a GDScript string literal contains the exact substring `# gdlint-ignore-next-line <rule>` (e.g. a help string), parse_ignore_patterns will create a spurious ignore directive for the following line, silently suppressing real lint warnings.","fix":"Use the tree-sitter AST to locate comment nodes rather than scanning raw source text for `#`. Alternatively, only scan characters after the last closing string delimiter on the line.","locations":[{"ref":"src/linter/ignore_patterns.rs:20-21","code":"   20 |         if let Some(comment_start) = line.find('#') {\n   21 |             let comment = &line[comment_start..];","lines":2}]},{"id":"LOGIC_29","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"`loop_variable_name.rs` only accepts `SNAKE_CASE` (starting with a lowercase letter,\nno leading underscore). A loop variable prefixed with `_` to indicate intentional\nnon-use (common GDScript pattern) would be flagged as an error even though such names\nare idiomatic. The `unused_argument` rule explicitly exempts `_`-prefixed names; loop\nvariables deserve the same treatment.","fix":"Accept `_` (single underscore, intentional discard) and PRIVATE_SNAKE_CASE as valid\nloop variable names, consistent with the unused-argument rule's treatment.","locations":[{"ref":"src/linter/rules/loop_variable_name.rs:10-12","code":"   10 |     fn is_valid_loop_variable_name(&self, name: &str) -> bool {\n   11 |         SNAKE_CASE.is_match(name)\n   12 |     }","lines":3},{"ref":"src/linter/rules/loop_variable_name.rs:40-52","code":"   40 |             if !variable_name.is_empty() && !self.is_valid_loop_variable_name(variable_name) {\n   41 |                 let (line, column) = get_line_column(&left_node);\n   42 |                 issues.push(LintIssue::new(\n   43 |                     line,\n   44 |                     column,\n   45 |                     \"loop-variable-name\".to_string(),\n   46 |                     LintSeverity::Error,\n   47 |                     format!(\n   48 |                         \"Loop variable '{}' should be in snake_case format\",\n   49 |                         variable_name\n   50 |                     ),\n   51 |                 ));\n   52 |             }","lines":13}]},{"id":"LOGIC_30","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"`private_access.rs` checks whether the object in an attribute access is literally the\nstring \"super\" or \"self\" by comparing text. If the variable holding a reference is\nnamed `self_ref` or `super_node`, it would be correctly flagged. However, if GDScript\ncode uses `self` stored in a variable with a different name, the access would be\nflagged even if it is within the same class. The real case is: `get_node_text` returns\nthe raw identifier text \"self\" or \"super\". This is correct for direct `self.` and\n`super.` accesses, but the rule cannot detect indirect self-access through aliases.\nThis is a known limitation (false positives for aliased self), not a crash, but worth\nnoting. More importantly: the rule checks `method_node.kind() == \"attribute_call\"` but\nthe AST uses this path only for `obj.method()` call syntax. Direct property access like\n`obj._field` would use the `identifier` branch. The code handles both branches, which\nis correct.","fix":"Document the known limitation that aliased self-access will produce false positives.\nNo structural fix needed unless the rule is extended to track type information.","locations":[{"ref":"src/linter/rules/private_access.rs:22-54","code":"   22 |                 if method_node.kind() == \"attribute_call\" {\n   23 |                     if let Some(method_name_node) = method_node.child(0) {\n   24 |                         let method_name = get_node_text(&method_name_node, source_code);\n   25 |                         if method_name.starts_with('_')\n   26 |                             && object_name != \"super\"\n   27 |                             && object_name != \"self\"\n   28 |                         {\n   29 |                             let (line, column) = get_line_column(&method_name_node);\n   30 |                             issues.push(LintIssue::new(\n   31 |                                 line,\n   32 |                                 column,\n   33 |                                 \"private-access\".to_string(),\n   34 |                                 LintSeverity::Error,\n   35 |                                 format!(\"Private method '{}' should not be called from outside its class\", method_name),\n   36 |                             ));\n   37 |                         }\n   38 |                     }\n   39 |                 } else if method_node.kind() == \"identifier\" {\n   40 |                     let method_name = get_node_text(&method_node, source_code);\n   41 |                     if method_name.starts_with('_')\n   42 |                         && object_name != \"super\"\n   43 |                         && object_name != \"self\"\n   44 |                     {\n   45 |                         let (line, column) = get_line_column(&method_node);\n   46 |                         issues.push(LintIssue::new(\n   47 |                             line,\n   48 |                             column,\n   49 |                             \"private-access\".to_string(),\n   50 |                             LintSeverity::Error,\n   51 |                             format!(\"Private variable '{}' should not be accessed from outside its class\", method_name),\n   52 |                         ));\n   53 |                     }\n   54 |                 }","lines":33}]},{"id":"LOGIC_31","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"`variable_name.rs` emits the rule name \"load-variable-name\" for variables whose value\nis a `load()` or `preload()` call but whose name is not in PascalCase/snake_case/\n_private_snake_case. However \"load-variable-name\" is not registered in `ALL_RULES`\nin rules.rs - only \"variable-name\" is. This means:\n1. Users cannot disable this rule via `--disable load-variable-name`.\n2. The rule name in lint output does not match any documented rule.\n3. `validate_rule_names` would reject \"load-variable-name\" if a user tried to ignore it.","fix":"Either register \"load-variable-name\" as a separate entry in ALL_RULES, or reuse the\n\"variable-name\" rule name in the issue emitted for load variables.","locations":[{"ref":"src/linter/rules/variable_name.rs:57-64","code":"   57 |                         \"load-variable-name\".to_string(),\n   58 |                         LintSeverity::Error,\n   59 |                         format!(\n   60 |                             \"Variable name '{}' should be in PascalCase, snake_case or _private_snake_case format\",\n   61 |                             name\n   62 |                         ),\n   63 |                     ));\n   64 |                 }","lines":8},{"ref":"src/linter/rules.rs:127-130","code":"  127 |         name: \"variable-name\",\n  128 |         create: |_config| Box::new(VariableNameRule),\n  129 |     },\n  130 |     RuleDefinition {","lines":4}]},{"id":"LOGIC_32","type":"logic_error","type_label":"Logic Error","severity":"low","easy":true,"description":"Two built-in virtual methods share priority 9: _unhandled_key_input and _gui_input. Since sort_by is stable, their relative order is preserved from the input, but the intent of the priority table (unique explicit ordering) is violated. All other methods have unique priorities.","fix":"Assign _gui_input a distinct priority (e.g. 10) and shift all following priorities up by one, adjusting the u8 values accordingly.","locations":[{"ref":"src/reorder.rs:25-26","code":"   25 |         \"_unhandled_key_input\" => 9,\n   26 |         \"_gui_input\" => 9,","lines":2}]},{"id":"LOGIC_33","type":"logic_error","type_label":"Logic Error","severity":"low","easy":true,"description":"`_unhandled_key_input` and `_gui_input` share the same priority value 9 in\n`get_builtin_virtual_priority` (lines 25-26). When both are present in a file, the sort\nis stable so their relative order is preserved from the source - but this makes the\nordering non-deterministic across different source files that define these methods in\ndifferent orders. It's unclear whether this sharing is intentional (they have equal\nGodot style-guide priority) or an accidental copy-paste. The docstring says \"assigns\npriority values\" implying each method should have a unique one.","fix":"If `_gui_input` intentionally has equal priority to `_unhandled_key_input`, add a comment\nexplaining why. If it should have a distinct priority (e.g. 10, shifting `_draw` to 11, etc.),\nassign the correct value and update the downstream numbers accordingly.","locations":[{"ref":"src/reorder.rs:25-26","code":"   25 |         \"_unhandled_key_input\" => 9,\n   26 |         \"_gui_input\" => 9,","lines":2}]},{"id":"LOGIC_34","type":"logic_error","type_label":"Logic Error","severity":"low","easy":true,"description":"GDScriptTokenKind::Unknown(_) is mapped to TokenKind::Method in get_token_kind (line 225). Unknown elements therefore receive the same double-newline spacing as functions, which is incorrect: an unknown top-level element such as an unrecognised declaration will always get two blank lines before it even if it appears adjacent to other variables.","fix":"Either add a TokenKind::Unknown variant to preserve original spacing intent, or map Unknown to TokenKind::Header so it at most gets one blank line separator.","locations":[{"ref":"src/reorder.rs:224-226","code":"  224 |         GDScriptTokenKind::InnerClass(_, _) => TokenKind::InnerClass,\n  225 |         GDScriptTokenKind::Unknown(_) => TokenKind::Method,\n  226 |     }","lines":3}]},{"id":"LOGIC_35","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"classify_element text-searches on class_name_statement for the substring \"extends\" to handle a legacy GDScript 3 combined declaration (\"class_name Foo extends Bar\"). The GDScript 4 grammar always produces separate class_name_statement and extends_statement nodes, so the splitn branch at line 629 is dead code in practice. Additionally, contains(\"extends\") would false-match a class named e.g. class_name MyExtends_Helper.","fix":"Remove the text-search branch. For GDScript 4, class_name and extends are always separate nodes. If GDScript 3 compatibility is needed, use tree-sitter child node inspection instead of string searching.","locations":[{"ref":"src/reorder.rs:625-640","code":"  625 |         \"class_name_statement\" => {\n  626 |             // If the class_name statement also has an extends in it, we split\n  627 |             // it into two separate elements on two lines.\n  628 |             if text.contains(\"extends\") {\n  629 |                 let parts: Vec<&str> = text.splitn(2, \"extends\").collect();\n  630 |                 if parts.len() == 2 {\n  631 |                     // We'll handle this case in the extraction logic\n  632 |                     Ok(Some(GDScriptTokenKind::ClassName(\n  633 |                         parts[0].trim().to_string(),\n  634 |                     )))\n  635 |                 } else {\n  636 |                     Ok(Some(GDScriptTokenKind::ClassName(text.to_string())))\n  637 |                 }\n  638 |             } else {\n  639 |                 Ok(Some(GDScriptTokenKind::ClassName(text.to_string())))\n  640 |             }","lines":16}]},{"id":"LOGIC_36","type":"logic_error","type_label":"Logic Error","severity":"low","easy":true,"description":"_static_init is always flagged as is_private=true because its name starts with '_' (line 665). _static_init is a Godot engine-reserved static initializer, not a user private method. The incorrect flag does not affect sorting order in practice (StaticInit has a unique priority 12) but is semantically wrong for any consumer reading token_kind.is_private().","fix":"Special-case _static_init: set is_private = false when name == \"_static_init\", or document that the is_private flag is intentionally set based on naming convention without exceptions.","locations":[{"ref":"src/reorder.rs:665-668","code":"  665 |             let is_private = name.starts_with('_');\n  666 | \n  667 |             let method_type = if name == \"_static_init\" {\n  668 |                 MethodType::StaticInit","lines":4}]},{"id":"LOGIC_37","type":"logic_error","type_label":"Logic Error","severity":"low","easy":false,"description":"In `build_reordered_code`, transitioning from any non-Method, non-InnerClass group to\nan InnerClass only emits a single `\\n` (line 948), not the two newlines (`\\n\\n`) that\nthe style guide requires around class definitions.\n\nThe `needs_spacing` check (line 919) correctly identifies this as a group transition and\nsets it to true. Then the spacing insertion block (lines 939-949) checks `is_function`\n(false) then `is_inner_class && previous == Method` (false) then\n`is_inner_class && previous == InnerClass` (false) and falls through to the `else` at\nline 947 that emits only one `\\n`.\n\nFor example, a file that has only exported variables followed by inner classes (no methods)\nwould produce only a single blank line before the inner class block instead of the\nexpected double-blank-line separation.","fix":"Add a case for `is_inner_class` without the previous-kind restriction:\n  } else if is_inner_class {\n      output.push_str(\"\\n\\n\");\n  } else {\n      output.push('\\n');\n  }\nThe existing two `is_inner_class` sub-cases can be removed as they become redundant.","locations":[{"ref":"src/reorder.rs:939-949","code":"  939 |         if needs_spacing {\n  940 |             #[allow(clippy::if_same_then_else)]\n  941 |             if is_function {\n  942 |                 output.push_str(\"\\n\\n\");\n  943 |             } else if is_inner_class && previous_token_kind == Some(TokenKind::Method) {\n  944 |                 output.push_str(\"\\n\\n\");\n  945 |             } else if is_inner_class && previous_token_kind == Some(TokenKind::InnerClass) {\n  946 |                 output.push_str(\"\\n\\n\");\n  947 |             } else {\n  948 |                 output.push('\\n');\n  949 |             }","lines":11}]},{"id":"ERR_1","type":"incorrect_error_handling","type_label":"Incorrect Error Handling","severity":"medium","easy":false,"description":"`find_gdscript_files` calls `std::process::exit(1)` when no .gd files are found\n(main.rs:394-395). This bypasses Rust's normal error propagation and prevents any\ncleanup (e.g., Drop implementations, deferred writes). The function signature returns\n`Result`, so it should return an `Err` and let `main` decide whether to exit. The\ncaller (`main`) already maps errors to `Box<dyn std::error::Error>`.","fix":"Replace `std::process::exit(1)` with:\n`return Err(\"No GDScript files found...\".into());`\nand let `main`'s error handler print and exit.","locations":[{"ref":"src/main.rs:390-395","code":"  390 |     if gdscript_file_paths.is_empty() {\n  391 |         eprintln!(\n  392 |             \"Error: No GDScript files found in the arguments provided. Please provide at least one .gd file or directory containing .gd files.\"\n  393 |         );\n  394 |         std::process::exit(1);\n  395 |     }","lines":6}]},{"id":"ERR_2","type":"incorrect_error_handling","type_label":"Incorrect Error Handling","severity":"low","easy":false,"description":"In `make_release.rs`, the `cargo_toml.replace(...)` call on line 141 replaces the\nfirst occurrence of `version = \"<current>\"` in Cargo.toml. If any dependency pin in\nthe file happens to use the exact same version string (e.g., a dep pinned to the same\nversion as the package), that line would also be modified, corrupting Cargo.toml.","fix":"Limit the replacement to the `[package]` section only, or use a regex that anchors\nthe match (e.g., ensuring the line starts with `version = ` at the beginning of the\npackage section). Alternatively, use a TOML parser.","locations":[{"ref":"src/bin/make_release.rs:141-144","code":"  141 |     let updated_cargo_toml = cargo_toml.replace(\n  142 |         &format!(\"version = \\\"{}\\\"\", current_version),\n  143 |         &format!(\"version = \\\"{}\\\"\", new_version),\n  144 |     );","lines":4}]},{"id":"PERF_1","type":"performance","type_label":"Performance","severity":"medium","easy":false,"description":"`fix_nested_parenthesized_lambda_indentation` (lines 315-462) splits the entire file\ncontent into a `Vec<String>` on line 339-343, where each line is a freshly allocated\n`String` (one allocation per line).  For a 10 000-line file this allocates 10 000\n`String` objects, then joins them back with `lines.join(\"\\n\")` at line 458 creating\nyet another large allocation.  This function is called on every format pass.\nA `Vec<&str>` with targeted in-place replacement of only modified lines would\navoid the bulk of these allocations.","fix":"Use `Vec<&str>` pointing into the original content for unmodified lines, and replace\nmodified lines with `Cow<str>` or a secondary buffer.  Only convert modified lines to\nowned Strings.","locations":[{"ref":"src/formatter.rs:339-343","code":"  339 |         let mut lines: Vec<String> = self\n  340 |             .content\n  341 |             .split('\\n')\n  342 |             .map(|line| line.to_string())\n  343 |             .collect();","lines":5},{"ref":"src/formatter.rs:458","code":"  458 |         self.content = lines.join(\"\\n\");","lines":1}]},{"id":"PERF_2","type":"performance","type_label":"Performance","severity":"medium","easy":false,"description":"`handle_two_blank_line` inserts newlines into the content string by calling `String::insert` at\narbitrary byte positions (lines 695, 700). Each `String::insert` call is O(n) in the length of the\nstring because all bytes after the insertion point must be shifted. The function may perform O(k)\nsuch insertions for k function/class pairs, resulting in O(k*n) total work for a file of size n.\nFor large files with many definitions this is noticeably slow.","fix":"Collect all the byte positions that need a newline, sort them in ascending order, then rebuild the\noutput string in a single O(n) pass, copying slices from the original content and inserting the\nnewlines between them. This avoids shifting content repeatedly.","locations":[{"ref":"src/formatter.rs:686-713","code":"  686 |         for (byte_idx, position) in new_lines_at {\n  687 |             let mut new_end_position = position;\n  688 |             let mut new_end_byte_idx = byte_idx;\n  689 |             // Only add a second blank line if there isn't already one\n  690 |             if !(self.content.as_bytes()[byte_idx] == b'\\n'\n  691 |                 && self.content.as_bytes()[byte_idx - 1] == b'\\n')\n  692 |             {\n  693 |                 new_end_position.row += 1;\n  694 |                 new_end_byte_idx += 1;\n  695 |                 self.content.insert(byte_idx, '\\n');\n  696 |             }\n  697 |             // Add the first blank line\n  698 |             new_end_position.row += 1;\n  699 |             new_end_byte_idx += 1;\n  700 |             self.content.insert(byte_idx, '\\n');\n  701 | \n  702 |             // Update the tree sitter parse tree to reflect our changes so that any\n  703 |             // future processing will work with the correct positions\n  704 |             self.tree.edit(&tree_sitter::InputEdit {\n  705 |                 start_byte: byte_idx,\n  706 |                 old_end_byte: byte_idx,\n  707 |                 new_end_byte: new_end_byte_idx,\n  708 |                 start_position: position,\n  709 |                 old_end_position: position,\n  710 |                 new_end_position,\n  711 |             });\n  712 |         }\n  713 |         self","lines":28}]},{"id":"PERF_3","type":"performance","type_label":"Performance","severity":"medium","easy":true,"description":"MaxLineLengthRule clones the entire LinterConfig (which includes a HashSet<String> of disabled rules) just to access a single usize field (max_line_length). The clone is done once per file, but the stored HashSet is never read by this rule.","fix":"Change the struct to `pub struct MaxLineLengthRule { max_line_length: usize }` and update `new` to store only `config.max_line_length`.","locations":[{"ref":"src/linter/rules/max_line_length.rs:4-13","code":"    4 | pub struct MaxLineLengthRule {\n    5 |     config: LinterConfig,\n    6 | }\n    7 | \n    8 | impl MaxLineLengthRule {\n    9 |     pub fn new(config: &LinterConfig) -> Self {\n   10 |         Self {\n   11 |             config: config.clone(),\n   12 |         }\n   13 |     }","lines":10}]},{"id":"PERF_4","type":"performance","type_label":"Performance","severity":"medium","easy":true,"description":"UnusedArgumentRule.is_identifier_used_in_node performs a full recursive tree walk for every function parameter separately. For a function with N parameters this is O(N * tree_size). A single pass collecting all used identifiers first would be O(tree_size + N).","fix":"Collect all identifier texts from the body into a HashSet<&str> in one traversal, then check each parameter name against that set.","locations":[{"ref":"src/linter/rules/unused_argument.rs:12-45","code":"   12 |     fn is_identifier_used_in_node(&self, node: &Node, identifier: &str, source_code: &str) -> bool {\n   13 |         let mut cursor = node.walk();\n   14 | \n   15 |         fn check_usage(\n   16 |             cursor: &mut tree_sitter::TreeCursor,\n   17 |             identifier: &str,\n   18 |             source_code: &str,\n   19 |         ) -> bool {\n   20 |             let node = cursor.node();\n   21 | \n   22 |             if node.kind() == \"identifier\" {\n   23 |                 let node_text = get_node_text(&node, source_code);\n   24 |                 if node_text == identifier {\n   25 |                     return true;\n   26 |                 }\n   27 |             }\n   28 | \n   29 |             if cursor.goto_first_child() {\n   30 |                 loop {\n   31 |                     if check_usage(cursor, identifier, source_code) {\n   32 |                         return true;\n   33 |                     }\n   34 |                     if !cursor.goto_next_sibling() {\n   35 |                         break;\n   36 |                     }\n   37 |                 }\n   38 |                 cursor.goto_parent();\n   39 |             }\n   40 | \n   41 |             false\n   42 |         }\n   43 | \n   44 |         check_usage(&mut cursor, identifier, source_code)\n   45 |     }","lines":34},{"ref":"src/linter/rules/unused_argument.rs:88-101","code":"   88 |         if let Some(body_node) = node.child_by_field_name(\"body\") {\n   89 |             for (param_name, param_node) in parameters {\n   90 |                 if !self.is_identifier_used_in_node(&body_node, &param_name, source_code) {\n   91 |                     let (line, column) = get_line_column(&param_node);\n   92 |                     issues.push(LintIssue::new(\n   93 |                         line,\n   94 |                         column,\n   95 |                         \"unused-argument\".to_string(),\n   96 |                         LintSeverity::Warning,\n   97 |                         format!(\"Function argument '{}' is unused. Consider removing it or prefixing with '_'\", param_name),\n   98 |                     ));\n   99 |                 }\n  100 |             }\n  101 |         }","lines":14}]},{"id":"PERF_5","type":"performance","type_label":"Performance","severity":"low","easy":true,"description":"In benchmark.rs the mutable `start` variable is declared once and reused for all four benchmark phases (lines 34, 42, 58, 68). While not incorrect, reusing a single variable across four unrelated timings makes the code harder to read and more fragile - a future insertion of code between phases might accidentally time the wrong interval.","fix":"Use a fresh `let start = Instant::now()` binding at the start of each benchmark phase instead of reassigning the same `mut start`.","locations":[{"ref":"src/bin/benchmark.rs:34-72","code":"   34 |     let mut start = Instant::now();\n   35 |     for _ in 0..ITERATIONS {\n   36 |         let _ = format_gdscript_with_config(&short_content, &config)?;\n   37 |     }\n   38 |     let duration_short_file = start.elapsed();\n   39 | \n   40 |     // Benchmark long file\n   41 |     println!(\"Benchmarking long file ({} iterations)...\", ITERATIONS);\n   42 |     start = Instant::now();\n   43 |     for _ in 0..ITERATIONS {\n   44 |         let _ = format_gdscript_with_config(&long_content, &config)?;\n   45 |     }\n   46 |     let long_time = start.elapsed();\n   47 | \n   48 |     // Benchmark with safe mode enabled\n   49 |     let safe_config = FormatterConfig {\n   50 |         safe: true,\n   51 |         ..config\n   52 |     };\n   53 | \n   54 |     println!(\n   55 |         \"Benchmarking short file with safe mode ({} iterations)...\",\n   56 |         ITERATIONS\n   57 |     );\n   58 |     start = Instant::now();\n   59 |     for _ in 0..ITERATIONS {\n   60 |         let _ = format_gdscript_with_config(&short_content, &safe_config)?;\n   61 |     }\n   62 |     let duration_short_file_safe = start.elapsed();\n   63 | \n   64 |     println!(\n   65 |         \"Benchmarking long file with safe mode ({} iterations)...\",\n   66 |         ITERATIONS\n   67 |     );\n   68 |     start = Instant::now();\n   69 |     for _ in 0..ITERATIONS {\n   70 |         let _ = format_gdscript_with_config(&long_content, &safe_config)?;\n   71 |     }\n   72 |     let long_time_safe = start.elapsed();","lines":39}]},{"id":"PERF_6","type":"performance","type_label":"Performance","severity":"low","easy":false,"description":"`fix_nested_parenthesized_lambda_indentation` splits the entire content into `Vec<String>` on line\n338-343, operates on the lines, then joins them back on line 458. Each `line.to_string()` call is a\nheap allocation, so for a 1000-line file this creates 1000 small `String` objects and then drops\nthem all. While this runs only when the content contains lambda nodes, on large files with many\nlambdas the allocation count is proportional to the file size.","fix":"Work with byte slices and offsets into the original content string rather than converting each line\nto an owned `String`. Alternatively, collect into `Vec<&str>` (referencing into `self.content`)\nand only allocate for the lines that actually need modification.","locations":[{"ref":"src/formatter.rs:338-343","code":"  338 | \n  339 |         let mut lines: Vec<String> = self\n  340 |             .content\n  341 |             .split('\\n')\n  342 |             .map(|line| line.to_string())\n  343 |             .collect();","lines":6}]},{"id":"PERF_7","type":"performance","type_label":"Performance","severity":"low","easy":false,"description":"`regex_replace_all_outside_strings_or_comments` calls `self.parser.parse(...)` on line\n579 after every single regex substitution, even when no replacements were actually made\n(the early-return on line 516-518 catches the zero-matches case, but not the case\nwhere all matches were skipped because they were inside strings/comments).  An\nincremental re-parse triggered with no real changes wastes cycles.","fix":"Track whether any replacement was actually applied (a boolean flag set inside the loop\nwhen a replacement is accepted) and skip `self.tree.edit` / `self.parser.parse` if the\nflag is false.","locations":[{"ref":"src/formatter.rs:576-579","code":"  576 |         for edit in edits {\n  577 |             self.tree.edit(&edit);\n  578 |         }\n  579 |         self.tree = self.parser.parse(&self.content, Some(&self.tree)).unwrap();","lines":4}]},{"id":"PERF_8","type":"performance","type_label":"Performance","severity":"low","easy":false,"description":"Inside `handle_two_blank_line`, the `process_query` closure calls `Query::new(...)` (lines 606-613) on every invocation, which compiles the query string against the grammar each time. This closure is invoked twice per call to `handle_two_blank_line` (once per query in `queries`), meaning two `Query` objects are compiled on every single format operation. These queries are static strings and could be compiled once.","fix":"Move the `Query::new` calls outside the per-format path, e.g. as `OnceLock<Query>` statics or by passing pre-built `Query` objects into the closure.","locations":[{"ref":"src/formatter.rs:604-614","code":"  604 |         let process_query =\n  605 |             |query_str: &str, new_lines_at: &mut Vec<(usize, tree_sitter::Point)>| {\n  606 |                 let query = match Query::new(\n  607 |                     &tree_sitter::Language::new(tree_sitter_gdscript::LANGUAGE),\n  608 |                     query_str,\n  609 |                 ) {\n  610 |                     Ok(q) => q,\n  611 |                     Err(err) => {\n  612 |                         panic!(\"Failed to create query: {}\", err);\n  613 |                     }\n  614 |                 };","lines":11}]},{"id":"PERF_9","type":"performance","type_label":"Performance","severity":"low","easy":false,"description":"`handle_two_blank_line` calls `String::insert(byte_idx, '\\n')` (lines 695 and 700) in\na loop.  `String::insert` is O(n) because it must shift all bytes after the insertion\npoint.  For a file with many function definitions this loop can run many times, making\nthe total complexity O(k*n) where k is the number of insertion points.  The positions\nare processed in reverse order (sorted descending on line 684), which limits damage,\nbut each individual insert is still O(n).  For very large files this is measurable.","fix":"Collect all insertion points in reverse order (already done), then rebuild the string\nin a single pass: iterate through the original bytes, inserting the required newlines\nat each tracked offset using a pre-allocated output buffer.","locations":[{"ref":"src/formatter.rs:684-712","code":"  684 |         new_lines_at.sort_by(|a, b| b.cmp(a));\n  685 | \n  686 |         for (byte_idx, position) in new_lines_at {\n  687 |             let mut new_end_position = position;\n  688 |             let mut new_end_byte_idx = byte_idx;\n  689 |             // Only add a second blank line if there isn't already one\n  690 |             if !(self.content.as_bytes()[byte_idx] == b'\\n'\n  691 |                 && self.content.as_bytes()[byte_idx - 1] == b'\\n')\n  692 |             {\n  693 |                 new_end_position.row += 1;\n  694 |                 new_end_byte_idx += 1;\n  695 |                 self.content.insert(byte_idx, '\\n');\n  696 |             }\n  697 |             // Add the first blank line\n  698 |             new_end_position.row += 1;\n  699 |             new_end_byte_idx += 1;\n  700 |             self.content.insert(byte_idx, '\\n');\n  701 | \n  702 |             // Update the tree sitter parse tree to reflect our changes so that any\n  703 |             // future processing will work with the correct positions\n  704 |             self.tree.edit(&tree_sitter::InputEdit {\n  705 |                 start_byte: byte_idx,\n  706 |                 old_end_byte: byte_idx,\n  707 |                 new_end_byte: new_end_byte_idx,\n  708 |                 start_position: position,\n  709 |                 old_end_position: position,\n  710 |                 new_end_position,\n  711 |             });\n  712 |         }","lines":29}]},{"id":"PERF_10","type":"performance","type_label":"Performance","severity":"low","easy":false,"description":"In `lint_files_pretty` (linter.rs:186), `file_issues` is a `HashMap<String, Vec<_>>`.\nThe file path string is inserted as a key via `.to_string()`. Then later, the map is\niterated and the path is used for both display and as a key for `line_issues.get()`.\nThe iteration order of `HashMap` is random, so the output order of files in pretty mode\nis non-deterministic across runs. This is inconsistent with `lint_files_standard` which\nprocesses files in the order they were passed in.","fix":"Collect into a `Vec<(String, Vec<LintIssue>)>` preserving insertion order, or sort the\nfile_issues before printing. This makes pretty output deterministic.","locations":[{"ref":"src/linter.rs:186-254","code":"  186 |         let mut file_issues: HashMap<String, Vec<_>> = HashMap::new();\n  187 |         let mut has_issues = false;\n  188 | \n  189 |         for file_path in gdscript_files {\n  190 |             let source_code = fs::read_to_string(file_path).map_err(|error| {\n  191 |                 format!(\"Failed to read file {}: {}\", file_path.display(), error)\n  192 |             })?;\n  193 | \n  194 |             let issues = self.lint(&source_code, &file_path.to_string_lossy())?;\n  195 | \n  196 |             if !issues.is_empty() {\n  197 |                 has_issues = true;\n  198 |                 file_issues.insert(file_path.to_string_lossy().to_string(), issues);\n  199 |             }\n  200 |         }\n  201 | \n  202 |         // Print pretty output grouped by file and line\n  203 |         let mut file_iter = file_issues.iter().peekable();\n  204 |         while let Some((file_path, issues)) = file_iter.next() {\n  205 |             let bold = if with_colors { \"\\x1b[1m\" } else { \"\" };\n  206 |             let reset = if with_colors { \"\\x1b[0m\" } else { \"\" };\n  207 | \n  208 |             println!(\"{}{}{}\", bold, file_path, reset);\n  209 | \n  210 |             // Group issues by line number\n  211 |             let mut line_issues: HashMap<usize, Vec<_>> = HashMap::new();\n  212 |             for issue in issues {\n  213 |                 line_issues.entry(issue.line).or_default().push(issue);\n  214 |             }\n  215 | \n  216 |             // Sort by line number and print\n  217 |             let mut sorted_lines: Vec<_> = line_issues.keys().collect();\n  218 |             sorted_lines.sort();\n  219 | \n  220 |             for (i, &line_num) in sorted_lines.iter().enumerate() {\n  221 |                 if let Some(line_issues) = line_issues.get(line_num) {\n  222 |                     println!(\"    {}:{}\", file_path, line_num);\n  223 |                     for issue in line_issues {\n  224 |                         let (severity_str, severity_color) = match issue.severity {\n  225 |                             LintSeverity::Error => (\"ERROR\", \"\\x1b[31m\"),  // Red\n  226 |                             LintSeverity::Warning => (\"WARN\", \"\\x1b[33m\"), // Yellow\n  227 |                         };\n  228 | \n  229 |                         if with_colors {\n  230 |                             println!(\n  231 |                                 \"        {}{}\\x1b[0m: `{}`\",\n  232 |                                 severity_color, severity_str, issue.rule\n  233 |                             );\n  234 |                         } else {\n  235 |                             println!(\"        {}: `{}`\", severity_str, issue.rule);\n  236 |                         }\n  237 |                         println!(\"        {}\", issue.message);\n  238 |                     }\n  239 | \n  240 |                     // Add newline between line groups (except for the last line group)\n  241 |                     if i < sorted_lines.len() - 1 {\n  242 |                         println!();\n  243 |                     }\n  244 |                 }\n  245 |             }\n  246 | \n  247 |             // Add separator between files (except for the last file)\n  248 |             if file_iter.peek().is_some() {\n  249 |                 println!(\"\\n{}\", \"-\".repeat(60));\n  250 |                 println!();\n  251 |             }\n  252 |         }\n  253 | \n  254 |         Ok(has_issues)","lines":69}]},{"id":"PERF_11","type":"performance","type_label":"Performance","severity":"low","easy":false,"description":"`MaxLineLengthRule` stores a full clone of `LinterConfig` just to access\n`max_line_length: usize`. The clone happens once per linting run but copies the entire\n`LinterConfig` struct (including a `HashSet<String>`) unnecessarily.","fix":"Store only the needed field: replace `config: LinterConfig` with\n`max_line_length: usize` and set it from `config.max_line_length` in `new()`.","locations":[{"ref":"src/linter/rules/max_line_length.rs:4-14","code":"    4 | pub struct MaxLineLengthRule {\n    5 |     config: LinterConfig,\n    6 | }\n    7 | \n    8 | impl MaxLineLengthRule {\n    9 |     pub fn new(config: &LinterConfig) -> Self {\n   10 |         Self {\n   11 |             config: config.clone(),\n   12 |         }\n   13 |     }\n   14 | }","lines":11}]},{"id":"PERF_12","type":"performance","type_label":"Performance","severity":"low","easy":false,"description":"Each top-level node's text is stored and cloned multiple times:\n1. Stored as `String` in `nodes_top_level_definitions` (line 263).\n2. Cloned again into `classified_elements` (line 337) via `text.clone()`.\n3. Used as `classified.text` in the main loop and moved into element structs.\nFor large functions (hundreds or thousands of lines), each GDScript top-level function\nbody is fully copied into memory twice before being placed in the output elements.\nWith `rayon` parallel processing in main.rs, multiple large files are processed\nsimultaneously, multiplying peak memory usage.","fix":"Merge the two passes (docstring collection and classification) into a single pass over\n`nodes_top_level_definitions`, or store text only once and reference it via slices\ninto the original `content` string (using byte offsets already available from tree-sitter\nnodes) rather than allocating new Strings for each element.","locations":[{"ref":"src/reorder.rs:260-264","code":"  260 |     let mut nodes_top_level_definitions = Vec::new();\n  261 |     for child in root.children(&mut cursor) {\n  262 |         let text = child.utf8_text(content.as_bytes())?;\n  263 |         nodes_top_level_definitions.push((child, text.to_string()));\n  264 |     }","lines":5},{"ref":"src/reorder.rs:325-338","code":"  325 |     let mut classified_elements = Vec::new();\n  326 |     // Here we associate comments and annotations with the next declaration. We\n  327 |     // loop through the node tree from top to bottom, collecting comments and\n  328 |     // annotations until we hit a declaration, at which point we attach the\n  329 |     // collected comments/annotations to that declaration.\n  330 |     for (node, text) in &nodes_top_level_definitions {\n  331 |         let is_before_class_declaration = node.start_byte() < first_class_declaration_byte;\n  332 |         let reorderable_element =\n  333 |             classify_element(*node, text, content, is_before_class_declaration)?;\n  334 |         classified_elements.push(ClassifiedElement {\n  335 |             node: *node,\n  336 |             text: text.clone(),\n  337 |             reorderable_element,\n  338 |         });","lines":14}]},{"id":"PERF_13","type":"performance","type_label":"Performance","severity":"low","easy":true,"description":"`class_docstring_comments_rows.contains(&node.start_position().row)` (line 360) is an\nO(n) linear scan called for every comment node in the file during the main classification\nloop. For files with many comments and long class docstrings this is O(comments *\ndocstring_rows). While the comment in the code acknowledges this (\"may look inefficient\"),\nthe fix is straightforward.","fix":"Replace the Vec with a HashSet<usize> for `class_docstring_comments_rows`. The set is\npopulated only once (lines 293-294 in the docstring-collection loop) and used only for\nmembership tests, so a HashSet is a direct drop-in that reduces the per-comment cost to O(1).","locations":[{"ref":"src/reorder.rs:360","code":"  360 |                 if class_docstring_comments_rows.contains(&node.start_position().row) {","lines":1}]},{"id":"PERF_14","type":"performance","type_label":"Performance","severity":"low","easy":true,"description":"For function_definition nodes, extract_function_name (line 663) calls node.utf8_text() internally, then is_static_method (line 664) calls node.utf8_text() a second time. Two identical UTF-8 extractions on the same node per function.","fix":"Extract the text once in classify_element and pass it to both extract_function_name and is_static_method, or inline is_static_method into classify_element using the already-available text variable.","locations":[{"ref":"src/reorder.rs:663-664","code":"  663 |             let name = extract_function_name(node, content)?;\n  664 |             let is_static = is_static_method(node, content);","lines":2},{"ref":"src/reorder.rs:795-796","code":"  795 | fn extract_function_name(node: Node, content: &str) -> Result<String, Box<dyn std::error::Error>> {\n  796 |     let text = node.utf8_text(content.as_bytes())?;","lines":2},{"ref":"src/reorder.rs:823-824","code":"  823 | fn is_static_method(node: Node, content: &str) -> bool {\n  824 |     let text = node.utf8_text(content.as_bytes()).unwrap_or(\"\");","lines":2}]},{"id":"PERF_15","type":"performance","type_label":"Performance","severity":"low","easy":true,"description":"classify_variable_statement calls node.utf8_text() at line 700 to get the text, then calls extract_variable_name(node, content) at line 701, which calls node.utf8_text() again internally at line 781. Two UTF-8 text extractions on the same tree-sitter node for every variable statement.","fix":"Accept the already-extracted text as a &str parameter in extract_variable_name (and the other extract_* helpers) rather than re-extracting from the node.","locations":[{"ref":"src/reorder.rs:700-701","code":"  700 |     let text = node.utf8_text(content.as_bytes())?;\n  701 |     let variable_name = extract_variable_name(node, content)?;","lines":2},{"ref":"src/reorder.rs:781","code":"  781 |     let text = node.utf8_text(content.as_bytes())?;","lines":1}]},{"id":"DEAD_1","type":"dead_code","type_label":"Dead Code","severity":"low","easy":true,"description":"`ask_user_input` in `make_release.rs` (line 18) uses an explicit `return` statement as the last\nexpression in the function body. This is not idiomatic Rust (the final expression is automatically\nreturned) and triggers `clippy::needless_return`. Additionally, the extra `return` statement makes\nthe function slightly harder to read as an expression-oriented function.","fix":"Remove the `return` keyword so the last line becomes just `input.trim().to_owned()`.","locations":[{"ref":"src/bin/make_release.rs:17-19","code":"   17 |         .expect(\"Failed to read user input\");\n   18 |     return input.trim().to_owned();\n   19 | }","lines":3}]},{"id":"DEAD_2","type":"dead_code","type_label":"Dead Code","severity":"low","easy":true,"description":"The `parent_id` field of `GdTreeNode` is written in the constructor and in `move_extends_statement`\n/ `move_annotations`, but is never read anywhere in the codebase. It takes memory in every node and\nadds a branch in the mutation code but provides no observable behavior.","fix":"Remove the `parent_id: Option<usize>` field from `GdTreeNode` and remove all assignments to it.","locations":[{"ref":"src/formatter.rs:1131-1137","code":" 1131 | struct GdTreeNode {\n 1132 |     parent_id: Option<usize>,\n 1133 |     grammar_id: u16,\n 1134 |     grammar_name: &'static str,\n 1135 |     text: Option<String>,\n 1136 |     children: Vec<usize>,\n 1137 | }","lines":7}]},{"id":"DEAD_3","type":"dead_code","type_label":"Dead Code","severity":"low","easy":true,"description":"The `column` field of LintIssue is captured and stored for every issue, and it is used as a tiebreaker in the sort (linter.rs:161), but it is never included in any user-visible output: not in LintIssue::format, not in lint_files_pretty, not in lint_files_standard, and not in the integration-test format string. Users have no way to see which column a lint issue appears on.","fix":"Either include the column in the output format (e.g. `file:line:col:rule:severity: message` consistent with most linters), or remove the column field if it is genuinely not needed.","locations":[{"ref":"src/linter.rs:27","code":"   27 |     pub column: usize,","lines":1},{"ref":"src/linter.rs:50-59","code":"   50 |     pub fn format(&self, file_path: &str) -> String {\n   51 |         let severity_str = match self.severity {\n   52 |             LintSeverity::Error => \"error\",\n   53 |             LintSeverity::Warning => \"warning\",\n   54 |         };\n   55 |         format!(\n   56 |             \"{}:{}:{}:{}: {}\",\n   57 |             file_path, self.line, self.rule, severity_str, self.message\n   58 |         )\n   59 |     }","lines":10}]},{"id":"DEAD_4","type":"dead_code","type_label":"Dead Code","severity":"low","easy":false,"description":"`LintIssue::column` is stored and sorted on (linter.rs:161) but never actually\nincluded in the standard output format. `LintIssue::format()` produces\n`file:line:rule:severity: message` - the column field is absent. The column is\nstored, sorted on, and used in pretty output (linter.rs:222), but the standard\nmachine-readable format omits it. This deviates from conventional linter output\nformats (e.g., `file:line:col:severity: message`) and wastes the stored data.","fix":"Include column in the standard format string: `{}:{}:{}:{}:{}: {}` with\n`self.column` added after `self.line`. Or document intentionally omitting it.","locations":[{"ref":"src/linter.rs:50-59","code":"   50 |     pub fn format(&self, file_path: &str) -> String {\n   51 |         let severity_str = match self.severity {\n   52 |             LintSeverity::Error => \"error\",\n   53 |             LintSeverity::Warning => \"warning\",\n   54 |         };\n   55 |         format!(\n   56 |             \"{}:{}:{}:{}: {}\",\n   57 |             file_path, self.line, self.rule, severity_str, self.message\n   58 |         )\n   59 |     }","lines":10},{"ref":"src/linter.rs:161","code":"  161 |         issues.sort_by(|a, b| a.line.cmp(&b.line).then(a.column.cmp(&b.column)));","lines":1}]},{"id":"DEAD_5","type":"dead_code","type_label":"Dead Code","severity":"low","easy":true,"description":"In `classify_element`, the else branch at lines 635-637 for the `class_name_statement`\nwith `extends` case is unreachable. `splitn(2, \"extends\")` on a string that contains\n\"extends\" always produces exactly 2 parts, so `parts.len() == 2` is always true when\nexecution reaches the if/else. The else branch `Ok(Some(GDScriptTokenKind::ClassName(text.to_string())))`\nis never executed.","fix":"Remove the `if parts.len() == 2` guard and the else branch; directly use `parts[0].trim()`.\nOr simplify the whole block with `text.split_once(\"extends\").map(|(p, _)| p.trim())`.","locations":[{"ref":"src/reorder.rs:629-637","code":"  629 |                 let parts: Vec<&str> = text.splitn(2, \"extends\").collect();\n  630 |                 if parts.len() == 2 {\n  631 |                     // We'll handle this case in the extraction logic\n  632 |                     Ok(Some(GDScriptTokenKind::ClassName(\n  633 |                         parts[0].trim().to_string(),\n  634 |                     )))\n  635 |                 } else {\n  636 |                     Ok(Some(GDScriptTokenKind::ClassName(text.to_string())))\n  637 |                 }","lines":9}]},{"id":"DEAD_6","type":"dead_code","type_label":"Dead Code","severity":"low","easy":true,"description":"The BuiltinVirtual sub-comparison in sort_gdscript_tokens is unreachable dead code. MethodType derives Ord, so type_a.cmp(type_b) at line 845 already handles BuiltinVirtual(p_a).cmp(BuiltinVirtual(p_b)) by comparing the inner u8 priority. If type_cmp is Equal at line 846 both variants must be identical BuiltinVirtual with the same inner value, so the inner if-let block at lines 851-858 can never return a non-Equal result.","fix":"Remove lines 850-858 entirely. The type_a.cmp(type_b) at line 845 already sorts BuiltinVirtual methods by their inner priority value.","locations":[{"ref":"src/reorder.rs:850-858","code":"  850 |             // For built-in virtual methods, we sort them by our priority list\n  851 |             if let (MethodType::BuiltinVirtual(p_a), MethodType::BuiltinVirtual(p_b)) =\n  852 |                 (type_a, type_b)\n  853 |             {\n  854 |                 let builtin_cmp = p_a.cmp(p_b);\n  855 |                 if builtin_cmp != std::cmp::Ordering::Equal {\n  856 |                     return builtin_cmp;\n  857 |                 }\n  858 |             }","lines":9}]},{"id":"DEAD_7","type":"dead_code","type_label":"Dead Code","severity":"low","easy":true,"description":"The inner BuiltinVirtual priority comparison block (lines 851-858) is unreachable dead code.\n`MethodType` derives `Ord`, so `type_a.cmp(type_b)` at line 845 already compares\n`BuiltinVirtual(p_a)` vs `BuiltinVirtual(p_b)` by their u8 field correctly via the derived\nimplementation. If `p_a != p_b`, the `return type_cmp` at line 847 fires before reaching\nthe inner block. If `p_a == p_b`, the inner block's comparison also returns `Equal` - the\nsame outcome as skipping it. The inner block can never produce a result different from\nsimply removing it.","fix":"Remove lines 851-858 entirely. The derived Ord on MethodType already handles all orderings\ncorrectly, including comparing the u8 priority inside BuiltinVirtual.","locations":[{"ref":"src/reorder.rs:851-858","code":"  851 |             if let (MethodType::BuiltinVirtual(p_a), MethodType::BuiltinVirtual(p_b)) =\n  852 |                 (type_a, type_b)\n  853 |             {\n  854 |                 let builtin_cmp = p_a.cmp(p_b);\n  855 |                 if builtin_cmp != std::cmp::Ordering::Equal {\n  856 |                     return builtin_cmp;\n  857 |                 }\n  858 |             }","lines":8}]},{"id":"DEAD_8","type":"dead_code","type_label":"Dead Code","severity":"negligible","easy":false,"description":"The `preprocess` method (lines 155-157) contains only `self` and is completely empty. It is called in the main formatting pipeline on line 41. The method serves no purpose in the current code and the surrounding doc-comment describes future intent that has never been implemented.","fix":"Either remove the method and its call site, or add a `// TODO` comment explaining the planned preprocessing steps to make the intent clear.","locations":[{"ref":"src/formatter.rs:151-157","code":"  151 |     /// This function runs over the content before going through topiary.\n  152 |     /// It is used to prepare the content for formatting or save performance by\n  153 |     /// pre-applying rules that could be performance-intensive through topiary.\n  154 |     #[inline(always)]\n  155 |     fn preprocess(&mut self) -> &mut Self {\n  156 |         self\n  157 |     }","lines":7}]},{"id":"DEAD_9","type":"dead_code","type_label":"Dead Code","severity":"negligible","easy":true,"description":"`GdTreeNode.text` (line 1135) is stored for every leaf node but is never read outside\nof `move_annotations` (where it reads the annotation name).  The `GdTree::from_ts_tree`\nfunction eagerly allocates a `String` for every leaf node in the tree (lines 913-920),\nwhich adds allocation pressure proportional to file size.  For the `safe` validation\npath (`validate_formatting`), the `text` field is only compared indirectly via the\n`PartialEq` implementation which does NOT compare `text` at all (lines 1089-1129).\nSo leaf text is allocated, stored, and compared zero times during `validate_formatting`.","fix":"Remove text allocation from `GdTree::from_ts_tree` and only extract annotation names\nlazily (directly from the source bytes via node byte range) in `move_annotations`.","locations":[{"ref":"src/formatter.rs:913-920","code":"  913 |                 let text = if ts_child.child(0).is_none() {\n  914 |                     let range = ts_child.range();\n  915 |                     Some(\n  916 |                         str::from_utf8(&source[range.start_byte..range.end_byte])\n  917 |                             .unwrap()\n  918 |                             .to_string(),\n  919 |                     )\n  920 |                 } else {","lines":8},{"ref":"src/formatter.rs:1089-1129","code":" 1089 | impl PartialEq for GdTree {\n 1090 |     fn eq(&self, other: &Self) -> bool {\n 1091 |         let mut left_stack = Vec::new();\n 1092 |         let mut right_stack = Vec::new();\n 1093 | \n 1094 |         // Starting from root (0)\n 1095 |         left_stack.push(0);\n 1096 |         right_stack.push(0);\n 1097 | \n 1098 |         while let (Some(left_current_node_id), Some(right_current_node_id)) =\n 1099 |             (left_stack.pop(), right_stack.pop())\n 1100 |         {\n 1101 |             let left_current_node = &self.nodes[left_current_node_id];\n 1102 |             let right_current_node = &other.nodes[right_current_node_id];\n 1103 |             if left_current_node.children.len() != right_current_node.children.len() {\n 1104 |                 // A different number of children means the syntax trees are different, so the code\n 1105 |                 // structure has changed.\n 1106 |                 // NOTE: There's a valid case of change: an annotation above a variable may be wrapped\n 1107 |                 // on the same line as the variable, which turns the annotation into a child of the variable.\n 1108 |                 // We could ignore this specific case, but for now, we consider any change in structure\n 1109 |                 // as a potential issue.\n 1110 |                 return false;\n 1111 |             }\n 1112 | \n 1113 |             for (left_node_id, right_node_id) in left_current_node\n 1114 |                 .children\n 1115 |                 .iter()\n 1116 |                 .zip(right_current_node.children.iter())\n 1117 |             {\n 1118 |                 let left_node = &self.nodes[*left_node_id];\n 1119 |                 let right_node = &other.nodes[*right_node_id];\n 1120 |                 if left_node.grammar_id != right_node.grammar_id {\n 1121 |                     return false;\n 1122 |                 }\n 1123 |                 left_stack.push(*left_node_id);\n 1124 |                 right_stack.push(*right_node_id);\n 1125 |             }\n 1126 |         }\n 1127 |         true\n 1128 |     }\n 1129 | }","lines":41}]},{"id":"DEAD_10","type":"dead_code","type_label":"Dead Code","severity":"negligible","easy":false,"description":"`IgnoreDirective` struct (ignore_patterns.rs:4-10) is defined with `pub` fields and\n`#[derive(Debug, Clone)]` but is never actually used anywhere. `parse_ignore_patterns`\nreturns a `HashMap<usize, HashSet<String>>` directly. The struct appears to be a leftover\nfrom an earlier design.","fix":"Remove the unused `IgnoreDirective` struct, or use it as the value type in the\n`ignore_map` to make the code more self-documenting.","locations":[{"ref":"src/linter/ignore_patterns.rs:4-10","code":"    4 | #[derive(Debug, Clone)]\n    5 | pub struct IgnoreDirective {\n    6 |     /// The line number this directive applies to\n    7 |     pub target_line: usize,\n    8 |     /// Set of rule names to ignore on this line\n    9 |     pub ignored_rules: HashSet<String>,\n   10 | }","lines":7}]},{"id":"CMT_1","type":"invalid_comment","type_label":"Invalid Comment","severity":"low","easy":true,"description":"The regex comment on line 219 contains the typo \"wtih\" instead of \"with\": \"must start wtih\n'extends' keyword\".","fix":"Change \"wtih\" to \"with\".","locations":[{"ref":"src/formatter.rs:219","code":"  219 |         // - must start wtih \"extends\" keyword","lines":1}]},{"id":"CMT_2","type":"invalid_comment","type_label":"Invalid Comment","severity":"low","easy":false,"description":"The FIXME comment in `postprocess_tree_sitter` (lines 491-496) states \"the regex-based steps that run before this ... can remove or reposition tokens, making the AST stale\". However, each call to `regex_replace_all_outside_strings_or_comments` already performs an incremental tree re-parse at line 579 (`self.tree = self.parser.parse(...)`). The AST is therefore not stale due to those steps; the actual reason the full re-parse is needed is uncertainty about whether the accumulated incremental edits are fully reliable. The comment misleads readers about the state of the tree.","fix":"Update the comment to accurately state: \"Each regex step does an incremental re-parse, but accumulated incremental hints may not be fully reliable for complex multi-step edits, so a full re-parse is done here for correctness.\"","locations":[{"ref":"src/formatter.rs:491-501","code":"  491 |         // FIXME: Nathan: A full re-parse is required here for now because the\n  492 |         // regex-based steps that run before this (fix_dangling_semicolons(),\n  493 |         // fix_dangling_commas(), etc.) can remove or reposition tokens, making\n  494 |         // the AST stale. handle_two_blank_line() runs tree-sitter queries\n  495 |         // against this tree, so it must reflect the actual content or it will\n  496 |         // compute wrong insertion points.\n  497 |         //\n  498 |         // If we're going to keep working on this, we should get rid of regexes\n  499 |         // and start moving everything to a visitor pattern that modifies the\n  500 |         // AST incrementally for formatting rules.\n  501 |         self.tree = self.parser.parse(&self.content, None).unwrap();","lines":11}]},{"id":"CMT_3","type":"invalid_comment","type_label":"Invalid Comment","severity":"low","easy":false,"description":"The comment on line 524 says: \"We first collect tree edits and then apply them,\nbecause regex returns positions from unmodified content.\"  This is misleading.  The\nedits are NOT collected and applied after the loop; instead, `new` (the output string)\nis built inside the loop, and `self.content` is updated on line 574 *before* the edits\nare applied on lines 576-578.  The tree edits are collected in `edits: Vec<...>` and\napplied afterward, but the string is built incrementally inside the loop.","fix":"Update the comment to accurately describe what happens:\n- The output string is built incrementally inside the loop.\n- The InputEdit structs are collected and applied to the tree after the loop (because\n  they use byte offsets into the *original* content that would be invalidated by\n  in-place modification).","locations":[{"ref":"src/formatter.rs:524-525","code":"  524 |         // We first collect tree edits and then apply them, because regex returns positions from unmodified content\n  525 |         let mut edits = Vec::new();","lines":2}]},{"id":"CMT_4","type":"invalid_comment","type_label":"Invalid Comment","severity":"low","easy":true,"description":"The comment on constant_name.rs:52 says \"For all load/preload constants, check load naming rules\" but the code only checks for preload (is_preload_call returns false for plain load()). The comment contradicts the actual behaviour.","fix":"Either fix the implementation to match the comment (handle load() too), or correct the comment to say \"For preload constants\" until the implementation is updated.","locations":[{"ref":"src/linter/rules/constant_name.rs:52","code":"   52 |                 // For all load/preload constants, check load naming rules","lines":1}]},{"id":"CMT_5","type":"invalid_comment","type_label":"Invalid Comment","severity":"low","easy":true,"description":"The comment at line 308 reads \"count how many rows are belong to the statement docstring\"\nwhich is grammatically incorrect (\"are belong\" should be \"belong\"). Minor but potentially\nconfusing for readers trying to understand the algorithm.","fix":"Change to \"count how many rows belong to the statement docstring\".","locations":[{"ref":"src/reorder.rs:308","code":"  308 |                     // count how many rows are belong to the statement docstring","lines":1}]},{"id":"CMT_6","type":"invalid_comment","type_label":"Invalid Comment","severity":"negligible","easy":false,"description":"In main.rs at line 221-222, the comment says \"The formatter runs largely single threaded\nso this speeds things up a lot on multi-core CPUs\". This is contradictory: something\nthat runs single-threaded would not benefit greatly from multi-core parallelism. The\nlikely intended meaning is that each file is formatted independently (no shared state\nbetween files), so rayon can run them in parallel. The comment misleads readers.","fix":"Rewrite as: \"Each file is formatted independently so rayon can process them in parallel\nacross CPU cores.\"","locations":[{"ref":"src/main.rs:221-223","code":"  221 |     // We use the rayon library to automatically process files in parallel for\n  222 |     // us. The formatter runs largely single threaded so this speeds things up a\n  223 |     // lot on multi-core CPUs","lines":3}]},{"id":"CMT_7","type":"invalid_comment","type_label":"Invalid Comment","severity":"negligible","easy":true,"description":"The comment on line 91 reads \"commentsa\" (typo for \"comments\") in the enum definition\nfor `Docstring`. Minor but misleading.","fix":"Change \"commentsa\" to \"comments\".","locations":[{"ref":"src/reorder.rs:91","code":"   91 |     Docstring(String), // Represents docstrings, commentsa that are above a declaration and start with ##","lines":1}]},{"id":"CMT_8","type":"invalid_comment","type_label":"Invalid Comment","severity":"negligible","easy":true,"description":"Typo in doc comment on GDScriptTokenKind::Docstring: \"commentsa\" should be \"comments\".","fix":"Change \"commentsa\" to \"comments\".","locations":[{"ref":"src/reorder.rs:91","code":"   91 |     Docstring(String), // Represents docstrings, commentsa that are above a declaration and start with ##","lines":1}]},{"id":"DUP_1","type":"duplicate_code","type_label":"Duplicate Code","severity":"medium","easy":true,"description":"lint_files_pretty (lines 189-199) and lint_files_standard (lines 263-276) duplicate the file-reading and linting loop: both do `fs::read_to_string -> self.lint -> collect results`. The only difference is what happens with the collected issues. This means any future change to file reading or error handling must be applied in two places.","fix":"Extract a shared `collect_issues(files) -> Result<Vec<(PathBuf, Vec<LintIssue>)>>` helper method, then call it from both lint_files_pretty and lint_files_standard.","locations":[{"ref":"src/linter.rs:189-199","code":"  189 |         for file_path in gdscript_files {\n  190 |             let source_code = fs::read_to_string(file_path).map_err(|error| {\n  191 |                 format!(\"Failed to read file {}: {}\", file_path.display(), error)\n  192 |             })?;\n  193 | \n  194 |             let issues = self.lint(&source_code, &file_path.to_string_lossy())?;\n  195 | \n  196 |             if !issues.is_empty() {\n  197 |                 has_issues = true;\n  198 |                 file_issues.insert(file_path.to_string_lossy().to_string(), issues);\n  199 |             }","lines":11},{"ref":"src/linter.rs:263-276","code":"  263 |         for file_path in gdscript_files {\n  264 |             let source_code = fs::read_to_string(file_path).map_err(|error| {\n  265 |                 format!(\"Failed to read file {}: {}\", file_path.display(), error)\n  266 |             })?;\n  267 | \n  268 |             let issues = self.lint(&source_code, &file_path.to_string_lossy())?;\n  269 | \n  270 |             for issue in issues {\n  271 |                 has_issues = true;\n  272 |                 println!(\"{}\", issue.format(&file_path.to_string_lossy()));\n  273 |             }\n  274 |         }\n  275 | \n  276 |         Ok(has_issues)","lines":14}]},{"id":"DUP_2","type":"duplicate_code","type_label":"Duplicate Code","severity":"medium","easy":false,"description":"`function_argument_name.rs` and `unused_argument.rs` both walk the function parameter\nlist in nearly identical ways: both iterate over `function_definition` -> `parameters`,\nboth match on `\"identifier\" | \"typed_parameter\" | \"default_parameter\" |\n\"typed_default_parameter\"`, and both extract the parameter name by checking\n`param_node.kind() == \"identifier\"` then falling back to `param_node.child(0)`.\nThis is ~25 lines of structurally identical code duplicated across two files.","fix":"Extract a helper function in `linter/lib.rs`, e.g.:\n`fn collect_param_names(params_node: &Node, source: &str) -> Vec<(&str, Node)>`\nand call it from both rules.","locations":[{"ref":"src/linter/rules/function_argument_name.rs:24-63","code":"   24 |         if let Some(params_node) = node.child_by_field_name(\"parameters\") {\n   25 |             let mut params_cursor = params_node.walk();\n   26 |             if params_cursor.goto_first_child() {\n   27 |                 loop {\n   28 |                     let param_node = params_cursor.node();\n   29 |                     if matches!(\n   30 |                         param_node.kind(),\n   31 |                         \"identifier\"\n   32 |                             | \"typed_parameter\"\n   33 |                             | \"default_parameter\"\n   34 |                             | \"typed_default_parameter\"\n   35 |                     ) {\n   36 |                         let param_name = if param_node.kind() == \"identifier\" {\n   37 |                             get_node_text(&param_node, source_code)\n   38 |                         } else if let Some(name_child) = param_node.child(0) {\n   39 |                             get_node_text(&name_child, source_code)\n   40 |                         } else {\n   41 |                             \"\"\n   42 |                         };\n   43 | \n   44 |                         if !param_name.is_empty() && !self.is_valid_argument_name(param_name) {\n   45 |                             let (line, column) = get_line_column(&param_node);\n   46 |                             issues.push(LintIssue::new(\n   47 |                                 line,\n   48 |                                 column,\n   49 |                                 \"function-argument-name\".to_string(),\n   50 |                                 LintSeverity::Error,\n   51 |                                 format!(\n   52 |                                     \"Function argument '{}' should be in snake_case or _private_snake_case format\",\n   53 |                                     param_name\n   54 |                                 ),\n   55 |                             ));\n   56 |                         }\n   57 |                     }\n   58 |                     if !params_cursor.goto_next_sibling() {\n   59 |                         break;\n   60 |                     }\n   61 |                 }\n   62 |             }\n   63 |         }","lines":40},{"ref":"src/linter/rules/unused_argument.rs:57-86","code":"   57 |         if let Some(params_node) = node.child_by_field_name(\"parameters\") {\n   58 |             let mut params_cursor = params_node.walk();\n   59 |             if params_cursor.goto_first_child() {\n   60 |                 loop {\n   61 |                     let param_node = params_cursor.node();\n   62 |                     if matches!(\n   63 |                         param_node.kind(),\n   64 |                         \"identifier\"\n   65 |                             | \"typed_parameter\"\n   66 |                             | \"default_parameter\"\n   67 |                             | \"typed_default_parameter\"\n   68 |                     ) {\n   69 |                         let param_name = if param_node.kind() == \"identifier\" {\n   70 |                             get_node_text(&param_node, source_code)\n   71 |                         } else if let Some(name_child) = param_node.child(0) {\n   72 |                             get_node_text(&name_child, source_code)\n   73 |                         } else {\n   74 |                             \"\"\n   75 |                         };\n   76 | \n   77 |                         if !param_name.is_empty() && !param_name.starts_with('_') {\n   78 |                             parameters.push((param_name.to_string(), param_node));\n   79 |                         }\n   80 |                     }\n   81 |                     if !params_cursor.goto_next_sibling() {\n   82 |                         break;\n   83 |                     }\n   84 |                 }\n   85 |             }\n   86 |         }","lines":30}]},{"id":"DUP_3","type":"duplicate_code","type_label":"Duplicate Code","severity":"low","easy":true,"description":"The Docstring element construction block is copy-pasted identically in two places (lines 517-527 and 495-506, both inside extract_tokens_to_reorder). Both blocks join class_docstring_comments with newlines, construct a GDScriptTokensWithComments with start_byte and end_byte both 0, and set class_docstring_attached. The only difference is the branch condition.","fix":"Extract a helper closure or function that emits the Docstring element and call it from both sites.","locations":[{"ref":"src/reorder.rs:494-506","code":"  494 |                     // Create separate docstring element if we have class docstrings\n  495 |                     if !class_docstring_attached && !class_docstring_comments.is_empty() {\n  496 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  497 |                         elements.push(GDScriptTokensWithComments {\n  498 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  499 |                             attached_comments: Vec::new(),\n  500 |                             trailing_comments: Vec::new(),\n  501 |                             original_text: docstring_text,\n  502 |                             start_byte: 0,\n  503 |                             end_byte: 0,\n  504 |                         });\n  505 |                         class_docstring_attached = true;\n  506 |                     }","lines":13},{"ref":"src/reorder.rs:513-527","code":"  513 |                     if !class_docstring_attached\n  514 |                         && !class_docstring_comments.is_empty()\n  515 |                         && !found_extends_declaration\n  516 |                     {\n  517 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  518 |                         elements.push(GDScriptTokensWithComments {\n  519 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  520 |                             attached_comments: Vec::new(),\n  521 |                             trailing_comments: Vec::new(),\n  522 |                             original_text: docstring_text,\n  523 |                             start_byte: 0,\n  524 |                             end_byte: 0,\n  525 |                         });\n  526 |                         class_docstring_attached = true;\n  527 |                     }","lines":15}]},{"id":"DUP_4","type":"duplicate_code","type_label":"Duplicate Code","severity":"low","easy":true,"description":"The logic to create a standalone Docstring element from `class_docstring_comments` is\nduplicated in two places (lines 495-506 and lines 513-527). Both blocks do:\n  1. join the vec with `\\n`\n  2. clone it for `token_kind` and `original_text`\n  3. push a `GDScriptTokensWithComments` with empty byte positions\n  4. set `class_docstring_attached = true`\nThe only difference is the surrounding condition checks.","fix":"Extract a helper function:\n  fn push_class_docstring(elements: &mut Vec<GDScriptTokensWithComments>, comments: &[String]) {\n      let text = comments.join(\"\\n\");\n      elements.push(GDScriptTokensWithComments {\n          token_kind: GDScriptTokenKind::Docstring(text.clone()),\n          attached_comments: Vec::new(),\n          trailing_comments: Vec::new(),\n          original_text: text,\n          start_byte: 0,\n          end_byte: 0,\n      });\n  }\nCall it from both sites.","locations":[{"ref":"src/reorder.rs:495-506","code":"  495 |                     if !class_docstring_attached && !class_docstring_comments.is_empty() {\n  496 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  497 |                         elements.push(GDScriptTokensWithComments {\n  498 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  499 |                             attached_comments: Vec::new(),\n  500 |                             trailing_comments: Vec::new(),\n  501 |                             original_text: docstring_text,\n  502 |                             start_byte: 0,\n  503 |                             end_byte: 0,\n  504 |                         });\n  505 |                         class_docstring_attached = true;\n  506 |                     }","lines":12},{"ref":"src/reorder.rs:513-527","code":"  513 |                     if !class_docstring_attached\n  514 |                         && !class_docstring_comments.is_empty()\n  515 |                         && !found_extends_declaration\n  516 |                     {\n  517 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  518 |                         elements.push(GDScriptTokensWithComments {\n  519 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  520 |                             attached_comments: Vec::new(),\n  521 |                             trailing_comments: Vec::new(),\n  522 |                             original_text: docstring_text,\n  523 |                             start_byte: 0,\n  524 |                             end_byte: 0,\n  525 |                         });\n  526 |                         class_docstring_attached = true;\n  527 |                     }","lines":15}]},{"id":"ARCH_1","type":"architecture","type_label":"Architecture","severity":"medium","easy":false,"description":"`handle_two_blank_line` (lines 587-714) is 127 lines long and embeds a large closure\n(`process_query`, lines 604-671) that itself contains complex tree-sitter match\ntraversal, comment classification logic, and byte-offset walking.  The blank-line\ninsertion loop (lines 686-712) is separate but tightly coupled.  The function mixes\nthree distinct concerns: (1) finding where to insert blank lines via tree-sitter\nqueries, (2) classifying whether a comment belongs to the next node or is an inline\ntrailing comment, and (3) mutating the string and incrementally updating the parse\ntree.  This makes it hard to test or reason about each concern in isolation.","fix":"Extract at minimum:\n- A standalone function `find_blank_line_insertion_points(tree, content) -> Vec<(usize, Point)>`\n  that encapsulates both queries and the comment classification.\n- The insertion loop stays in `handle_two_blank_line` but only calls the extractor.","locations":[{"ref":"src/formatter.rs:587-714","code":"  587 |     fn handle_two_blank_line(&mut self) -> &mut Self {\n  588 |         let root = self.tree.root_node();\n  589 |         let queries = [\n  590 |             // We need two queries to catch all cases because variables can be placed above or below functions\n  591 |             // First query: variable, function, class, signal, const, enum followed by function, constructor, class, or variable\n  592 |             //\n  593 |             // NOTE: Nathan (GDQuest): This adds maybe 20-25% runtime to the program.\n  594 |             // I tried 2 other implementations by having a single query that'd find only functions, classes, and constructors and add 2 new lines between them.\n  595 |             // But the costly part is in accounting for comments and annotations between them. This solution ends up being slightly faster and simpler.\n  596 |             // Still, this is probably something that can be made faster in the future.\n  597 |             \"(([(variable_statement) (function_definition) (class_definition) (signal_statement) (const_statement) (enum_definition) (constructor_definition)]) @first \\\n  598 |             . (([(comment) (annotation)])* @comment . ([(function_definition) (constructor_definition) (class_definition)]) @second))\",\n  599 |             // Second query: constructor or function followed by variable, signal, const, or enum\n  600 |             \"(([(constructor_definition) (function_definition) (class_definition)]) @first \\\n  601 |             . ([(variable_statement) (signal_statement) (const_statement) (enum_definition)]) @second)\",\n  602 |         ];\n  603 | \n  604 |         let process_query =\n  605 |             |query_str: &str, new_lines_at: &mut Vec<(usize, tree_sitter::Point)>| {\n  606 |                 let query = match Query::new(\n  607 |                     &tree_sitter::Language::new(tree_sitter_gdscript::LANGUAGE),\n  608 |                     query_str,\n  609 |                 ) {\n  610 |                     Ok(q) => q,\n  611 |                     Err(err) => {\n  612 |                         panic!(\"Failed to create query: {}\", err);\n  613 |                     }\n  614 |                 };\n  615 | \n  616 |                 let mut cursor = QueryCursor::new();\n  617 |                 let mut matches = cursor.matches(&query, root, self.content.as_bytes());\n  618 |                 while let Some(m) = matches.next() {\n  619 |                     let first_node = m.captures[0].node;\n  620 |                     let last_node = m.captures.last().unwrap().node;\n  621 | \n  622 |                     let mut insert_before = last_node;\n  623 | \n  624 |                     let capture_has_comments = m.captures.len() >= 3;\n  625 | \n  626 |                     if capture_has_comments {\n  627 |                         let last_comment_node = m.captures[m.captures.len() - 2].node;\n  628 | \n  629 |                         let last_comment_is_inline_comment = last_comment_node.start_position().row\n  630 |                             == first_node.start_position().row;\n  631 |                         let last_comment_is_doc_comment = !last_comment_is_inline_comment\n  632 |                             && last_comment_node.start_position().row\n  633 |                                 == last_node.start_position().row - 1;\n  634 | \n  635 |                         // if last comment node is a doc comment find first doc comment node and insert new lines before that\n  636 |                         if last_comment_is_doc_comment {\n  637 |                             let mut comment_node_index = m.captures.len() - 2;\n  638 | \n  639 |                             let first_comment_node = m.captures[1].node;\n  640 |                             let first_comment_is_inline_comment =\n  641 |                                 first_comment_node.start_position().row\n  642 |                                     == first_node.start_position().row;\n  643 |                             // ignore n first nodes when searching for the first docstring comment node\n  644 |                             // in case if the first comment is an inline comment we ignore\n  645 |                             // two nodes: first statement node and inline comment node\n  646 |                             // otherwise we ignore only the first statement node\n  647 |                             let mut amount_of_nodes_to_ignore = 1;\n  648 |                             if first_comment_is_inline_comment {\n  649 |                                 amount_of_nodes_to_ignore += 1;\n  650 |                             }\n  651 | \n  652 |                             // find first documentation comment node\n  653 |                             while comment_node_index > amount_of_nodes_to_ignore\n  654 |                                 && m.captures[comment_node_index - 1].node.start_position().row\n  655 |                                     == m.captures[comment_node_index].node.start_position().row - 1\n  656 |                             {\n  657 |                                 comment_node_index -= 1;\n  658 |                             }\n  659 |                             insert_before = m.captures[comment_node_index].node;\n  660 |                         }\n  661 |                     }\n  662 | \n  663 |                     let mut byte_idx = insert_before.start_byte();\n  664 |                     let mut position = insert_before.start_position();\n  665 |                     position.column = 0;\n  666 |                     while byte_idx > 0 && self.content.as_bytes()[byte_idx] != b'\\n' {\n  667 |                         byte_idx -= 1;\n  668 |                     }\n  669 |                     new_lines_at.push((byte_idx, position));\n  670 |                 }\n  671 |             };\n  672 | \n  673 |         // First we need to find all the places where we should add blank lines.\n  674 |         // We can't modify the content string while tree-sitter is borrowing it, so we\n  675 |         // collect all the positions first, then make changes afterward.\n  676 |         let mut new_lines_at = Vec::new();\n  677 | \n  678 |         for query_str in &queries {\n  679 |             process_query(query_str, &mut new_lines_at);\n  680 |         }\n  681 | \n  682 |         // We sort the positions in reverse order so that when we insert new lines,\n  683 |         // we don't mess up the positions of the other insertions we need to make.\n  684 |         new_lines_at.sort_by(|a, b| b.cmp(a));\n  685 | \n  686 |         for (byte_idx, position) in new_lines_at {\n  687 |             let mut new_end_position = position;\n  688 |             let mut new_end_byte_idx = byte_idx;\n  689 |             // Only add a second blank line if there isn't already one\n  690 |             if !(self.content.as_bytes()[byte_idx] == b'\\n'\n  691 |                 && self.content.as_bytes()[byte_idx - 1] == b'\\n')\n  692 |             {\n  693 |                 new_end_position.row += 1;\n  694 |                 new_end_byte_idx += 1;\n  695 |                 self.content.insert(byte_idx, '\\n');\n  696 |             }\n  697 |             // Add the first blank line\n  698 |             new_end_position.row += 1;\n  699 |             new_end_byte_idx += 1;\n  700 |             self.content.insert(byte_idx, '\\n');\n  701 | \n  702 |             // Update the tree sitter parse tree to reflect our changes so that any\n  703 |             // future processing will work with the correct positions\n  704 |             self.tree.edit(&tree_sitter::InputEdit {\n  705 |                 start_byte: byte_idx,\n  706 |                 old_end_byte: byte_idx,\n  707 |                 new_end_byte: new_end_byte_idx,\n  708 |                 start_position: position,\n  709 |                 old_end_position: position,\n  710 |                 new_end_position,\n  711 |             });\n  712 |         }\n  713 |         self\n  714 |     }","lines":128}]},{"id":"ARCH_2","type":"architecture","type_label":"Architecture","severity":"low","easy":false,"description":"`handle_two_blank_line` is a 130-line method (lines 587-714) that inlines two tree-sitter query\nexecutions via a closure, a forward pass to collect insertion points, and a backward-sorted insertion\nloop. The closure `process_query` captures `self` mutably but is called inside the method body,\npreventing the code from being extracted cleanly. The method comment already notes it costs 20-25%\nof total runtime. Mixing query execution, position collection, and content mutation in one method\nmakes it harder to test and optimize the parts independently.","fix":"Extract `process_query` into a standalone free function that takes `root`, `content`, and a mutable\n`Vec<(usize, Point)>` output parameter. Keep the insertion loop in the method. This separation\nallows the query logic to be unit-tested without a full `Formatter` object, and makes the hot path\neasier to profile and replace.","locations":[{"ref":"src/formatter.rs:587-714","code":"  587 |     fn handle_two_blank_line(&mut self) -> &mut Self {\n  588 |         let root = self.tree.root_node();\n  589 |         let queries = [\n  590 |             // We need two queries to catch all cases because variables can be placed above or below functions\n  591 |             // First query: variable, function, class, signal, const, enum followed by function, constructor, class, or variable\n  592 |             //\n  593 |             // NOTE: Nathan (GDQuest): This adds maybe 20-25% runtime to the program.\n  594 |             // I tried 2 other implementations by having a single query that'd find only functions, classes, and constructors and add 2 new lines between them.\n  595 |             // But the costly part is in accounting for comments and annotations between them. This solution ends up being slightly faster and simpler.\n  596 |             // Still, this is probably something that can be made faster in the future.\n  597 |             \"(([(variable_statement) (function_definition) (class_definition) (signal_statement) (const_statement) (enum_definition) (constructor_definition)]) @first \\\n  598 |             . (([(comment) (annotation)])* @comment . ([(function_definition) (constructor_definition) (class_definition)]) @second))\",\n  599 |             // Second query: constructor or function followed by variable, signal, const, or enum\n  600 |             \"(([(constructor_definition) (function_definition) (class_definition)]) @first \\\n  601 |             . ([(variable_statement) (signal_statement) (const_statement) (enum_definition)]) @second)\",\n  602 |         ];\n  603 | \n  604 |         let process_query =\n  605 |             |query_str: &str, new_lines_at: &mut Vec<(usize, tree_sitter::Point)>| {\n  606 |                 let query = match Query::new(\n  607 |                     &tree_sitter::Language::new(tree_sitter_gdscript::LANGUAGE),\n  608 |                     query_str,\n  609 |                 ) {\n  610 |                     Ok(q) => q,\n  611 |                     Err(err) => {\n  612 |                         panic!(\"Failed to create query: {}\", err);\n  613 |                     }\n  614 |                 };\n  615 | \n  616 |                 let mut cursor = QueryCursor::new();\n  617 |                 let mut matches = cursor.matches(&query, root, self.content.as_bytes());\n  618 |                 while let Some(m) = matches.next() {\n  619 |                     let first_node = m.captures[0].node;\n  620 |                     let last_node = m.captures.last().unwrap().node;\n  621 | \n  622 |                     let mut insert_before = last_node;\n  623 | \n  624 |                     let capture_has_comments = m.captures.len() >= 3;\n  625 | \n  626 |                     if capture_has_comments {\n  627 |                         let last_comment_node = m.captures[m.captures.len() - 2].node;\n  628 | \n  629 |                         let last_comment_is_inline_comment = last_comment_node.start_position().row\n  630 |                             == first_node.start_position().row;\n  631 |                         let last_comment_is_doc_comment = !last_comment_is_inline_comment\n  632 |                             && last_comment_node.start_position().row\n  633 |                                 == last_node.start_position().row - 1;\n  634 | \n  635 |                         // if last comment node is a doc comment find first doc comment node and insert new lines before that\n  636 |                         if last_comment_is_doc_comment {\n  637 |                             let mut comment_node_index = m.captures.len() - 2;\n  638 | \n  639 |                             let first_comment_node = m.captures[1].node;\n  640 |                             let first_comment_is_inline_comment =\n  641 |                                 first_comment_node.start_position().row\n  642 |                                     == first_node.start_position().row;\n  643 |                             // ignore n first nodes when searching for the first docstring comment node\n  644 |                             // in case if the first comment is an inline comment we ignore\n  645 |                             // two nodes: first statement node and inline comment node\n  646 |                             // otherwise we ignore only the first statement node\n  647 |                             let mut amount_of_nodes_to_ignore = 1;\n  648 |                             if first_comment_is_inline_comment {\n  649 |                                 amount_of_nodes_to_ignore += 1;\n  650 |                             }\n  651 | \n  652 |                             // find first documentation comment node\n  653 |                             while comment_node_index > amount_of_nodes_to_ignore\n  654 |                                 && m.captures[comment_node_index - 1].node.start_position().row\n  655 |                                     == m.captures[comment_node_index].node.start_position().row - 1\n  656 |                             {\n  657 |                                 comment_node_index -= 1;\n  658 |                             }\n  659 |                             insert_before = m.captures[comment_node_index].node;\n  660 |                         }\n  661 |                     }\n  662 | \n  663 |                     let mut byte_idx = insert_before.start_byte();\n  664 |                     let mut position = insert_before.start_position();\n  665 |                     position.column = 0;\n  666 |                     while byte_idx > 0 && self.content.as_bytes()[byte_idx] != b'\\n' {\n  667 |                         byte_idx -= 1;\n  668 |                     }\n  669 |                     new_lines_at.push((byte_idx, position));\n  670 |                 }\n  671 |             };\n  672 | \n  673 |         // First we need to find all the places where we should add blank lines.\n  674 |         // We can't modify the content string while tree-sitter is borrowing it, so we\n  675 |         // collect all the positions first, then make changes afterward.\n  676 |         let mut new_lines_at = Vec::new();\n  677 | \n  678 |         for query_str in &queries {\n  679 |             process_query(query_str, &mut new_lines_at);\n  680 |         }\n  681 | \n  682 |         // We sort the positions in reverse order so that when we insert new lines,\n  683 |         // we don't mess up the positions of the other insertions we need to make.\n  684 |         new_lines_at.sort_by(|a, b| b.cmp(a));\n  685 | \n  686 |         for (byte_idx, position) in new_lines_at {\n  687 |             let mut new_end_position = position;\n  688 |             let mut new_end_byte_idx = byte_idx;\n  689 |             // Only add a second blank line if there isn't already one\n  690 |             if !(self.content.as_bytes()[byte_idx] == b'\\n'\n  691 |                 && self.content.as_bytes()[byte_idx - 1] == b'\\n')\n  692 |             {\n  693 |                 new_end_position.row += 1;\n  694 |                 new_end_byte_idx += 1;\n  695 |                 self.content.insert(byte_idx, '\\n');\n  696 |             }\n  697 |             // Add the first blank line\n  698 |             new_end_position.row += 1;\n  699 |             new_end_byte_idx += 1;\n  700 |             self.content.insert(byte_idx, '\\n');\n  701 | \n  702 |             // Update the tree sitter parse tree to reflect our changes so that any\n  703 |             // future processing will work with the correct positions\n  704 |             self.tree.edit(&tree_sitter::InputEdit {\n  705 |                 start_byte: byte_idx,\n  706 |                 old_end_byte: byte_idx,\n  707 |                 new_end_byte: new_end_byte_idx,\n  708 |                 start_position: position,\n  709 |                 old_end_position: position,\n  710 |                 new_end_position,\n  711 |             });\n  712 |         }\n  713 |         self\n  714 |     }","lines":128}]},{"id":"ARCH_3","type":"architecture","type_label":"Architecture","severity":"low","easy":false,"description":"`lint_gdscript_with_config` (linter.rs:319-326) and `lint_gdscript` (linter.rs:328-331)\ncreate a fresh `GDScriptLinter` (including tree-sitter `Parser` initialization) for\nevery call. When used in a tight loop or from tests calling these free functions\nrepeatedly, the Parser is re-initialized every time. The `GDScriptLinter` struct was\nexplicitly designed to be reused (it holds the `Parser`), but these convenience\nfunctions defeat that goal.","fix":"Document that these convenience functions have per-call overhead and are intended for\none-off use. For batch linting, callers should use `GDScriptLinter` directly. Optionally\nadd a `#[doc]` warning. The integration tests already use `GDScriptLinter` directly.","locations":[{"ref":"src/linter.rs:319-331","code":"  319 | pub fn lint_gdscript_with_config(\n  320 |     source_code: &str,\n  321 |     file_path: &str,\n  322 |     config: &LinterConfig,\n  323 | ) -> Result<Vec<LintIssue>, String> {\n  324 |     let mut linter = GDScriptLinter::new(config.clone())?;\n  325 |     linter.lint(source_code, file_path)\n  326 | }\n  327 | \n  328 | pub fn lint_gdscript(source_code: &str, file_path: &str) -> Result<Vec<LintIssue>, String> {\n  329 |     let config = LinterConfig::default();\n  330 |     lint_gdscript_with_config(source_code, file_path, &config)\n  331 | }","lines":13}]},{"id":"ARCH_4","type":"architecture","type_label":"Architecture","severity":"low","easy":false,"description":"`extract_tokens_to_reorder` in `reorder.rs` is a ~360-line function (lines 248-608) that handles\nclass-docstring collection, inline-comment attachment, annotation grouping, `region_end` tracking,\nand the final trailing-comment flush -- all in a single pass. Each of these responsibilities has its\nown local state variables (`class_docstring_comments`, `pending_comments`, `pending_annotations`,\n`region_end_comment`, `class_docstring_attached`, `found_extends_declaration`). The interleaved\nlogic makes it difficult to understand the state transitions and add new node kinds safely.","fix":"Split the function into smaller helpers: one to collect the class docstring, one to classify and\ngroup elements (annotations + comments attached to the next declaration), and one to flush trailing\nstate. This allows each sub-pass to be tested independently.","locations":[{"ref":"src/reorder.rs:248-608","code":"  248 | fn extract_tokens_to_reorder(\n  249 |     tree: &Tree,\n  250 |     content: &str,\n  251 | ) -> Result<Vec<GDScriptTokensWithComments>, Box<dyn std::error::Error>> {\n  252 |     let root = tree.root_node();\n  253 |     let mut elements: Vec<GDScriptTokensWithComments> = Vec::new();\n  254 | \n  255 |     // Collect all direct children of the source node by walking the tree.\n  256 |     // We use `children(...)` rather than a tree-sitter query because queries\n  257 |     // can miss extras like ERROR nodes, causing malformed code to be silently\n  258 |     // dropped. Direct iteration captures everything.\n  259 |     let mut cursor = root.walk();\n  260 |     let mut nodes_top_level_definitions = Vec::new();\n  261 |     for child in root.children(&mut cursor) {\n  262 |         let text = child.utf8_text(content.as_bytes())?;\n  263 |         nodes_top_level_definitions.push((child, text.to_string()));\n  264 |     }\n  265 | \n  266 |     // First we process the top of the node tree. We look for the class docstring.\n  267 |     // For now we treat them as any ## comments that appear before any declaration\n  268 |     // like a variable or function. We collect them and then attach them to the\n  269 |     // extends statement if we find one.\n  270 |     //\n  271 |     // TODO: Nathan (GDQuest): this is not perfect, we need to handle more edge cases, but I'm\n  272 |     // pushing this for now to make the command more usable. We can improve this later.\n  273 |     // Notably a comment after the extends declaration might be a var or method docstring.\n  274 |     // We need to check if the comments are contiguous with the declaration they are\n  275 |     // attached to.\n  276 | \n  277 |     // Find the byte position of the first class_name or extends statement\n  278 |     // Any annotations before this position are class-level annotations\n  279 |     let first_class_declaration_byte = nodes_top_level_definitions\n  280 |         .iter()\n  281 |         .find(|(node, _)| {\n  282 |             node.kind() == \"class_name_statement\" || node.kind() == \"extends_statement\"\n  283 |         })\n  284 |         .map(|(node, _)| node.start_byte())\n  285 |         .unwrap_or(usize::MAX);\n  286 | \n  287 |     let mut class_docstring_comments = Vec::new();\n  288 |     let mut class_docstring_comments_rows = Vec::new();\n  289 |     for (node, text) in &nodes_top_level_definitions {\n  290 |         match node.kind() {\n  291 |             \"comment\" => {\n  292 |                 if text.trim_start().starts_with(\"##\") {\n  293 |                     class_docstring_comments.push(text.clone());\n  294 |                     class_docstring_comments_rows.push(node.start_position().row);\n  295 |                 }\n  296 |             }\n  297 |             \"class_name_statement\" | \"extends_statement\" | \"annotation\" => {\n  298 |                 continue;\n  299 |             }\n  300 |             // Any other element means we're past the top of the file, so we stop\n  301 |             // collecting the class docstring\n  302 |             _ => {\n  303 |                 // if the last node of the docstring is immediately followed by the current node\n  304 |                 if class_docstring_comments_rows\n  305 |                     .last()\n  306 |                     .is_some_and(|row| row + 1 == node.start_position().row)\n  307 |                 {\n  308 |                     // count how many rows are belong to the statement docstring\n  309 |                     let statement_docstring_rows = class_docstring_comments_rows\n  310 |                         .iter()\n  311 |                         .rev()\n  312 |                         .zip((0..).map(|i| class_docstring_comments_rows.last().unwrap() - i))\n  313 |                         .take_while(|(actual, expected)| **actual == *expected)\n  314 |                         .count();\n  315 |                     class_docstring_comments\n  316 |                         .truncate(class_docstring_comments.len() - statement_docstring_rows);\n  317 |                     class_docstring_comments_rows\n  318 |                         .truncate(class_docstring_comments_rows.len() - statement_docstring_rows);\n  319 |                 }\n  320 |                 break;\n  321 |             }\n  322 |         }\n  323 |     }\n  324 | \n  325 |     let mut classified_elements = Vec::new();\n  326 |     // Here we associate comments and annotations with the next declaration. We\n  327 |     // loop through the node tree from top to bottom, collecting comments and\n  328 |     // annotations until we hit a declaration, at which point we attach the\n  329 |     // collected comments/annotations to that declaration.\n  330 |     for (node, text) in &nodes_top_level_definitions {\n  331 |         let is_before_class_declaration = node.start_byte() < first_class_declaration_byte;\n  332 |         let reorderable_element =\n  333 |             classify_element(*node, text, content, is_before_class_declaration)?;\n  334 |         classified_elements.push(ClassifiedElement {\n  335 |             node: *node,\n  336 |             text: text.clone(),\n  337 |             reorderable_element,\n  338 |         });\n  339 |     }\n  340 |     let mut pending_comments = Vec::new();\n  341 |     let mut pending_annotations = Vec::new();\n  342 |     let mut found_extends_declaration = false;\n  343 |     let mut class_docstring_attached = false;\n  344 |     // TODO: Handle multiple #region/#endregion pairs properly\n  345 |     // Nathan: For now we just attach the last #endregion to the most recent function\n  346 |     // that has a #region comment, to handle the most common use case\n  347 |     // Regions generally are tricky to reorder as they can span multiple\n  348 |     // functions that should be reordered. In those cases I would recommend users not to\n  349 |     // use regions though, or not to use the reorder feature\n  350 |     let mut region_end_comment = None;\n  351 | \n  352 |     for classified in classified_elements {\n  353 |         let node = classified.node;\n  354 |         let text = classified.text;\n  355 |         let reorderable_element = classified.reorderable_element;\n  356 |         match node.kind() {\n  357 |             \"comment\" => {\n  358 |                 // We already processed class docstring comments, so we skip them here\n  359 |                 // This may look inefficient but in practice it should not have much impact\n  360 |                 if class_docstring_comments_rows.contains(&node.start_position().row) {\n  361 |                     continue;\n  362 |                 }\n  363 | \n  364 |                 // Here we look for inline comments after declarations, and if\n  365 |                 // so, we attach them as inline to the declaration.  For\n  366 |                 // example:\n  367 |                 //\n  368 |                 // var test = 1 # inline comment\n  369 |                 //\n  370 |                 // Without this code, the comment would wrap to the next line.\n  371 |                 let mut handled_inline = false;\n  372 |                 if let Some(last_element) = elements.last_mut() {\n  373 |                     let last_end = last_element.end_byte;\n  374 |                     let comment_start = node.start_byte();\n  375 |                     if last_end <= comment_start\n  376 |                         && comment_start <= content.len()\n  377 |                         && let Some(spacing) = content.get(last_end..comment_start)\n  378 |                     {\n  379 |                         let has_newline = spacing.contains('\\n') || spacing.contains('\\r');\n  380 |                         if !has_newline {\n  381 |                             last_element.original_text.push_str(spacing);\n  382 |                             last_element.original_text.push_str(&text);\n  383 |                             last_element.end_byte = node.end_byte();\n  384 |                             handled_inline = true;\n  385 |                         }\n  386 |                     }\n  387 |                 }\n  388 | \n  389 |                 if !handled_inline {\n  390 |                     pending_comments.push(PendingAttachment {\n  391 |                         start_byte: node.start_byte(),\n  392 |                         text: text.clone(),\n  393 |                     });\n  394 |                 }\n  395 |             }\n  396 |             \"region_start\" => {\n  397 |                 pending_comments.push(PendingAttachment {\n  398 |                     start_byte: node.start_byte(),\n  399 |                     text: text.clone(),\n  400 |                 });\n  401 |             }\n  402 |             \"region_end\" => {\n  403 |                 // Attach #endregion to the most recently seen function so that\n  404 |                 // when reordering the end stays with the last function in the\n  405 |                 // region. Because functions change order, that can still change\n  406 |                 // the region but it prevents issues with the end region jumping\n  407 |                 // to a different function\n  408 |                 let mut attached = false;\n  409 |                 for element in elements.iter_mut().rev() {\n  410 |                     if matches!(element.token_kind, GDScriptTokenKind::Method(_, _, _)) {\n  411 |                         element.trailing_comments.push(text.clone());\n  412 |                         attached = true;\n  413 |                         break;\n  414 |                     }\n  415 |                 }\n  416 |                 if !attached {\n  417 |                     // We didn't find a function to attach to, so we save this\n  418 |                     // to handle down below\n  419 |                     region_end_comment = Some(text.clone());\n  420 |                 }\n  421 |             }\n  422 |             \"annotation\" => {\n  423 |                 if let Some(element) = reorderable_element {\n  424 |                     match element {\n  425 |                         GDScriptTokenKind::ClassAnnotation(_) => {\n  426 |                             elements.push(GDScriptTokensWithComments {\n  427 |                                 token_kind: element,\n  428 |                                 attached_comments: Vec::new(),\n  429 |                                 trailing_comments: Vec::new(),\n  430 |                                 original_text: text,\n  431 |                                 start_byte: node.start_byte(),\n  432 |                                 end_byte: node.end_byte(),\n  433 |                             });\n  434 |                         }\n  435 |                         _ => {\n  436 |                             pending_annotations.push(PendingAttachment {\n  437 |                                 start_byte: node.start_byte(),\n  438 |                                 text: text.clone(),\n  439 |                             });\n  440 |                         }\n  441 |                     }\n  442 |                 } else {\n  443 |                     pending_annotations.push(PendingAttachment {\n  444 |                         start_byte: node.start_byte(),\n  445 |                         text: text.clone(),\n  446 |                     });\n  447 |                 }\n  448 |             }\n  449 |             \"class_name_statement\" => {\n  450 |                 if let Some(element) = reorderable_element {\n  451 |                     // Don't attach class docstring to class_name, save it for extends\n  452 |                     let mut attachments: Vec<&PendingAttachment> =\n  453 |                         pending_annotations.iter().collect();\n  454 |                     attachments.extend(pending_comments.iter());\n  455 |                     attachments.sort_by_key(|attachment| attachment.start_byte);\n  456 |                     let non_docstring_comments: Vec<String> = attachments\n  457 |                         .into_iter()\n  458 |                         .filter_map(|attachment| {\n  459 |                             if !class_docstring_comments.contains(&attachment.text) {\n  460 |                                 Some(attachment.text.clone())\n  461 |                             } else {\n  462 |                                 None\n  463 |                             }\n  464 |                         })\n  465 |                         .collect();\n  466 |                     elements.push(GDScriptTokensWithComments {\n  467 |                         token_kind: element,\n  468 |                         attached_comments: non_docstring_comments,\n  469 |                         trailing_comments: Vec::new(),\n  470 |                         original_text: text,\n  471 |                         start_byte: node.start_byte(),\n  472 |                         end_byte: node.end_byte(),\n  473 |                     });\n  474 |                     pending_comments.clear();\n  475 |                     pending_annotations.clear();\n  476 |                 }\n  477 |             }\n  478 |             \"extends_statement\" => {\n  479 |                 found_extends_declaration = true;\n  480 |                 if let Some(element) = reorderable_element {\n  481 |                     let combined_comments =\n  482 |                         merge_pending_texts(&pending_annotations, &pending_comments);\n  483 |                     elements.push(GDScriptTokensWithComments {\n  484 |                         token_kind: element,\n  485 |                         attached_comments: combined_comments,\n  486 |                         trailing_comments: Vec::new(),\n  487 |                         original_text: text,\n  488 |                         start_byte: node.start_byte(),\n  489 |                         end_byte: node.end_byte(),\n  490 |                     });\n  491 |                     pending_comments.clear();\n  492 |                     pending_annotations.clear();\n  493 | \n  494 |                     // Create separate docstring element if we have class docstrings\n  495 |                     if !class_docstring_attached && !class_docstring_comments.is_empty() {\n  496 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  497 |                         elements.push(GDScriptTokensWithComments {\n  498 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  499 |                             attached_comments: Vec::new(),\n  500 |                             trailing_comments: Vec::new(),\n  501 |                             original_text: docstring_text,\n  502 |                             start_byte: 0,\n  503 |                             end_byte: 0,\n  504 |                         });\n  505 |                         class_docstring_attached = true;\n  506 |                     }\n  507 |                 }\n  508 |             }\n  509 |             _ => {\n  510 |                 if let Some(element) = reorderable_element {\n  511 |                     // If we haven't attached class docstring yet and this is the first real element,\n  512 |                     // create a separate docstring element (for cases where there's no extends)\n  513 |                     if !class_docstring_attached\n  514 |                         && !class_docstring_comments.is_empty()\n  515 |                         && !found_extends_declaration\n  516 |                     {\n  517 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  518 |                         elements.push(GDScriptTokensWithComments {\n  519 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  520 |                             attached_comments: Vec::new(),\n  521 |                             trailing_comments: Vec::new(),\n  522 |                             original_text: docstring_text,\n  523 |                             start_byte: 0,\n  524 |                             end_byte: 0,\n  525 |                         });\n  526 |                         class_docstring_attached = true;\n  527 |                     }\n  528 | \n  529 |                     let combined_comments =\n  530 |                         merge_pending_texts(&pending_annotations, &pending_comments);\n  531 | \n  532 |                     elements.push(GDScriptTokensWithComments {\n  533 |                         token_kind: element,\n  534 |                         attached_comments: combined_comments,\n  535 |                         trailing_comments: Vec::new(),\n  536 |                         original_text: text,\n  537 |                         start_byte: node.start_byte(),\n  538 |                         end_byte: node.end_byte(),\n  539 |                     });\n  540 |                     pending_comments.clear();\n  541 |                     pending_annotations.clear();\n  542 |                 } else {\n  543 |                     // We create unknown element for unhandled nodes to preserve\n  544 |                     // them. Given how the module works, if we don't do that the\n  545 |                     // nodes will be dropped.\n  546 |                     let combined_comments =\n  547 |                         merge_pending_texts(&pending_annotations, &pending_comments);\n  548 |                     elements.push(GDScriptTokensWithComments {\n  549 |                         token_kind: GDScriptTokenKind::Unknown(text.clone()),\n  550 |                         attached_comments: combined_comments,\n  551 |                         trailing_comments: Vec::new(),\n  552 |                         original_text: text,\n  553 |                         start_byte: node.start_byte(),\n  554 |                         end_byte: node.end_byte(),\n  555 |                     });\n  556 |                     pending_comments.clear();\n  557 |                     pending_annotations.clear();\n  558 |                 }\n  559 |             }\n  560 |         }\n  561 |     }\n  562 | \n  563 |     // `region_end_comment` stores a trailing `#endregion` that should follow\n  564 |     // the most recent function with a matching `#region`. If we found no\n  565 |     // matching function, we fall back to attaching it to the last element (or\n  566 |     // create a standalone element at the end). This avoids the formatter\n  567 |     // deleting or losing the directive when we reorder code blocks.\n  568 |     if let Some(region_end) = region_end_comment.take() {\n  569 |         if let Some(last_element) = elements.last_mut() {\n  570 |             last_element.trailing_comments.push(region_end);\n  571 |         } else {\n  572 |             elements.push(GDScriptTokensWithComments {\n  573 |                 token_kind: GDScriptTokenKind::Unknown(region_end.clone()),\n  574 |                 attached_comments: Vec::new(),\n  575 |                 trailing_comments: Vec::new(),\n  576 |                 original_text: region_end,\n  577 |                 start_byte: 0,\n  578 |                 end_byte: 0,\n  579 |             });\n  580 |         }\n  581 |     }\n  582 | \n  583 |     // Comments and annotations that we have not yet attached go at the end of\n  584 |     // the file. They are either trailing comments, stray `#region` markers, or\n  585 |     // unknown statements that appear after the last declaration (like new\n  586 |     // syntax in a dev version of Godot). We merge them in source order with\n  587 |     // `merge_pending_texts` and append them either to the last reordered\n  588 |     // element or to a dedicated \"Unknown\" token so the text remains in the\n  589 |     // output.\n  590 |     if !pending_annotations.is_empty() || !pending_comments.is_empty() {\n  591 |         let trailing_texts = merge_pending_texts(&pending_annotations, &pending_comments);\n  592 |         if let Some(last_element) = elements.last_mut() {\n  593 |             last_element.trailing_comments.extend(trailing_texts);\n  594 |         } else if !trailing_texts.is_empty() {\n  595 |             let combined_text = trailing_texts.join(\"\\n\");\n  596 |             elements.push(GDScriptTokensWithComments {\n  597 |                 token_kind: GDScriptTokenKind::Unknown(combined_text.clone()),\n  598 |                 attached_comments: Vec::new(),\n  599 |                 trailing_comments: Vec::new(),\n  600 |                 original_text: combined_text,\n  601 |                 start_byte: 0,\n  602 |                 end_byte: 0,\n  603 |             });\n  604 |         }\n  605 |     }\n  606 | \n  607 |     Ok(elements)\n  608 | }","lines":361}]},{"id":"ARCH_5","type":"architecture","type_label":"Architecture","severity":"low","easy":false,"description":"extract_tokens_to_reorder is a ~360-line function with multiple responsibilities: tree traversal, class-docstring boundary detection, statement-docstring stripping from class-docstring, annotation/comment buffering, element classification, and inline-comment detection. This makes it hard to test individual phases and understand the control flow.","fix":"Split into smaller focused functions: one for finding the class-docstring boundary, one for the main node-visiting loop, and one for the post-loop cleanup (region_end, trailing comments).","locations":[{"ref":"src/reorder.rs:248-608","code":"  248 | fn extract_tokens_to_reorder(\n  249 |     tree: &Tree,\n  250 |     content: &str,\n  251 | ) -> Result<Vec<GDScriptTokensWithComments>, Box<dyn std::error::Error>> {\n  252 |     let root = tree.root_node();\n  253 |     let mut elements: Vec<GDScriptTokensWithComments> = Vec::new();\n  254 | \n  255 |     // Collect all direct children of the source node by walking the tree.\n  256 |     // We use `children(...)` rather than a tree-sitter query because queries\n  257 |     // can miss extras like ERROR nodes, causing malformed code to be silently\n  258 |     // dropped. Direct iteration captures everything.\n  259 |     let mut cursor = root.walk();\n  260 |     let mut nodes_top_level_definitions = Vec::new();\n  261 |     for child in root.children(&mut cursor) {\n  262 |         let text = child.utf8_text(content.as_bytes())?;\n  263 |         nodes_top_level_definitions.push((child, text.to_string()));\n  264 |     }\n  265 | \n  266 |     // First we process the top of the node tree. We look for the class docstring.\n  267 |     // For now we treat them as any ## comments that appear before any declaration\n  268 |     // like a variable or function. We collect them and then attach them to the\n  269 |     // extends statement if we find one.\n  270 |     //\n  271 |     // TODO: Nathan (GDQuest): this is not perfect, we need to handle more edge cases, but I'm\n  272 |     // pushing this for now to make the command more usable. We can improve this later.\n  273 |     // Notably a comment after the extends declaration might be a var or method docstring.\n  274 |     // We need to check if the comments are contiguous with the declaration they are\n  275 |     // attached to.\n  276 | \n  277 |     // Find the byte position of the first class_name or extends statement\n  278 |     // Any annotations before this position are class-level annotations\n  279 |     let first_class_declaration_byte = nodes_top_level_definitions\n  280 |         .iter()\n  281 |         .find(|(node, _)| {\n  282 |             node.kind() == \"class_name_statement\" || node.kind() == \"extends_statement\"\n  283 |         })\n  284 |         .map(|(node, _)| node.start_byte())\n  285 |         .unwrap_or(usize::MAX);\n  286 | \n  287 |     let mut class_docstring_comments = Vec::new();\n  288 |     let mut class_docstring_comments_rows = Vec::new();\n  289 |     for (node, text) in &nodes_top_level_definitions {\n  290 |         match node.kind() {\n  291 |             \"comment\" => {\n  292 |                 if text.trim_start().starts_with(\"##\") {\n  293 |                     class_docstring_comments.push(text.clone());\n  294 |                     class_docstring_comments_rows.push(node.start_position().row);\n  295 |                 }\n  296 |             }\n  297 |             \"class_name_statement\" | \"extends_statement\" | \"annotation\" => {\n  298 |                 continue;\n  299 |             }\n  300 |             // Any other element means we're past the top of the file, so we stop\n  301 |             // collecting the class docstring\n  302 |             _ => {\n  303 |                 // if the last node of the docstring is immediately followed by the current node\n  304 |                 if class_docstring_comments_rows\n  305 |                     .last()\n  306 |                     .is_some_and(|row| row + 1 == node.start_position().row)\n  307 |                 {\n  308 |                     // count how many rows are belong to the statement docstring\n  309 |                     let statement_docstring_rows = class_docstring_comments_rows\n  310 |                         .iter()\n  311 |                         .rev()\n  312 |                         .zip((0..).map(|i| class_docstring_comments_rows.last().unwrap() - i))\n  313 |                         .take_while(|(actual, expected)| **actual == *expected)\n  314 |                         .count();\n  315 |                     class_docstring_comments\n  316 |                         .truncate(class_docstring_comments.len() - statement_docstring_rows);\n  317 |                     class_docstring_comments_rows\n  318 |                         .truncate(class_docstring_comments_rows.len() - statement_docstring_rows);\n  319 |                 }\n  320 |                 break;\n  321 |             }\n  322 |         }\n  323 |     }\n  324 | \n  325 |     let mut classified_elements = Vec::new();\n  326 |     // Here we associate comments and annotations with the next declaration. We\n  327 |     // loop through the node tree from top to bottom, collecting comments and\n  328 |     // annotations until we hit a declaration, at which point we attach the\n  329 |     // collected comments/annotations to that declaration.\n  330 |     for (node, text) in &nodes_top_level_definitions {\n  331 |         let is_before_class_declaration = node.start_byte() < first_class_declaration_byte;\n  332 |         let reorderable_element =\n  333 |             classify_element(*node, text, content, is_before_class_declaration)?;\n  334 |         classified_elements.push(ClassifiedElement {\n  335 |             node: *node,\n  336 |             text: text.clone(),\n  337 |             reorderable_element,\n  338 |         });\n  339 |     }\n  340 |     let mut pending_comments = Vec::new();\n  341 |     let mut pending_annotations = Vec::new();\n  342 |     let mut found_extends_declaration = false;\n  343 |     let mut class_docstring_attached = false;\n  344 |     // TODO: Handle multiple #region/#endregion pairs properly\n  345 |     // Nathan: For now we just attach the last #endregion to the most recent function\n  346 |     // that has a #region comment, to handle the most common use case\n  347 |     // Regions generally are tricky to reorder as they can span multiple\n  348 |     // functions that should be reordered. In those cases I would recommend users not to\n  349 |     // use regions though, or not to use the reorder feature\n  350 |     let mut region_end_comment = None;\n  351 | \n  352 |     for classified in classified_elements {\n  353 |         let node = classified.node;\n  354 |         let text = classified.text;\n  355 |         let reorderable_element = classified.reorderable_element;\n  356 |         match node.kind() {\n  357 |             \"comment\" => {\n  358 |                 // We already processed class docstring comments, so we skip them here\n  359 |                 // This may look inefficient but in practice it should not have much impact\n  360 |                 if class_docstring_comments_rows.contains(&node.start_position().row) {\n  361 |                     continue;\n  362 |                 }\n  363 | \n  364 |                 // Here we look for inline comments after declarations, and if\n  365 |                 // so, we attach them as inline to the declaration.  For\n  366 |                 // example:\n  367 |                 //\n  368 |                 // var test = 1 # inline comment\n  369 |                 //\n  370 |                 // Without this code, the comment would wrap to the next line.\n  371 |                 let mut handled_inline = false;\n  372 |                 if let Some(last_element) = elements.last_mut() {\n  373 |                     let last_end = last_element.end_byte;\n  374 |                     let comment_start = node.start_byte();\n  375 |                     if last_end <= comment_start\n  376 |                         && comment_start <= content.len()\n  377 |                         && let Some(spacing) = content.get(last_end..comment_start)\n  378 |                     {\n  379 |                         let has_newline = spacing.contains('\\n') || spacing.contains('\\r');\n  380 |                         if !has_newline {\n  381 |                             last_element.original_text.push_str(spacing);\n  382 |                             last_element.original_text.push_str(&text);\n  383 |                             last_element.end_byte = node.end_byte();\n  384 |                             handled_inline = true;\n  385 |                         }\n  386 |                     }\n  387 |                 }\n  388 | \n  389 |                 if !handled_inline {\n  390 |                     pending_comments.push(PendingAttachment {\n  391 |                         start_byte: node.start_byte(),\n  392 |                         text: text.clone(),\n  393 |                     });\n  394 |                 }\n  395 |             }\n  396 |             \"region_start\" => {\n  397 |                 pending_comments.push(PendingAttachment {\n  398 |                     start_byte: node.start_byte(),\n  399 |                     text: text.clone(),\n  400 |                 });\n  401 |             }\n  402 |             \"region_end\" => {\n  403 |                 // Attach #endregion to the most recently seen function so that\n  404 |                 // when reordering the end stays with the last function in the\n  405 |                 // region. Because functions change order, that can still change\n  406 |                 // the region but it prevents issues with the end region jumping\n  407 |                 // to a different function\n  408 |                 let mut attached = false;\n  409 |                 for element in elements.iter_mut().rev() {\n  410 |                     if matches!(element.token_kind, GDScriptTokenKind::Method(_, _, _)) {\n  411 |                         element.trailing_comments.push(text.clone());\n  412 |                         attached = true;\n  413 |                         break;\n  414 |                     }\n  415 |                 }\n  416 |                 if !attached {\n  417 |                     // We didn't find a function to attach to, so we save this\n  418 |                     // to handle down below\n  419 |                     region_end_comment = Some(text.clone());\n  420 |                 }\n  421 |             }\n  422 |             \"annotation\" => {\n  423 |                 if let Some(element) = reorderable_element {\n  424 |                     match element {\n  425 |                         GDScriptTokenKind::ClassAnnotation(_) => {\n  426 |                             elements.push(GDScriptTokensWithComments {\n  427 |                                 token_kind: element,\n  428 |                                 attached_comments: Vec::new(),\n  429 |                                 trailing_comments: Vec::new(),\n  430 |                                 original_text: text,\n  431 |                                 start_byte: node.start_byte(),\n  432 |                                 end_byte: node.end_byte(),\n  433 |                             });\n  434 |                         }\n  435 |                         _ => {\n  436 |                             pending_annotations.push(PendingAttachment {\n  437 |                                 start_byte: node.start_byte(),\n  438 |                                 text: text.clone(),\n  439 |                             });\n  440 |                         }\n  441 |                     }\n  442 |                 } else {\n  443 |                     pending_annotations.push(PendingAttachment {\n  444 |                         start_byte: node.start_byte(),\n  445 |                         text: text.clone(),\n  446 |                     });\n  447 |                 }\n  448 |             }\n  449 |             \"class_name_statement\" => {\n  450 |                 if let Some(element) = reorderable_element {\n  451 |                     // Don't attach class docstring to class_name, save it for extends\n  452 |                     let mut attachments: Vec<&PendingAttachment> =\n  453 |                         pending_annotations.iter().collect();\n  454 |                     attachments.extend(pending_comments.iter());\n  455 |                     attachments.sort_by_key(|attachment| attachment.start_byte);\n  456 |                     let non_docstring_comments: Vec<String> = attachments\n  457 |                         .into_iter()\n  458 |                         .filter_map(|attachment| {\n  459 |                             if !class_docstring_comments.contains(&attachment.text) {\n  460 |                                 Some(attachment.text.clone())\n  461 |                             } else {\n  462 |                                 None\n  463 |                             }\n  464 |                         })\n  465 |                         .collect();\n  466 |                     elements.push(GDScriptTokensWithComments {\n  467 |                         token_kind: element,\n  468 |                         attached_comments: non_docstring_comments,\n  469 |                         trailing_comments: Vec::new(),\n  470 |                         original_text: text,\n  471 |                         start_byte: node.start_byte(),\n  472 |                         end_byte: node.end_byte(),\n  473 |                     });\n  474 |                     pending_comments.clear();\n  475 |                     pending_annotations.clear();\n  476 |                 }\n  477 |             }\n  478 |             \"extends_statement\" => {\n  479 |                 found_extends_declaration = true;\n  480 |                 if let Some(element) = reorderable_element {\n  481 |                     let combined_comments =\n  482 |                         merge_pending_texts(&pending_annotations, &pending_comments);\n  483 |                     elements.push(GDScriptTokensWithComments {\n  484 |                         token_kind: element,\n  485 |                         attached_comments: combined_comments,\n  486 |                         trailing_comments: Vec::new(),\n  487 |                         original_text: text,\n  488 |                         start_byte: node.start_byte(),\n  489 |                         end_byte: node.end_byte(),\n  490 |                     });\n  491 |                     pending_comments.clear();\n  492 |                     pending_annotations.clear();\n  493 | \n  494 |                     // Create separate docstring element if we have class docstrings\n  495 |                     if !class_docstring_attached && !class_docstring_comments.is_empty() {\n  496 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  497 |                         elements.push(GDScriptTokensWithComments {\n  498 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  499 |                             attached_comments: Vec::new(),\n  500 |                             trailing_comments: Vec::new(),\n  501 |                             original_text: docstring_text,\n  502 |                             start_byte: 0,\n  503 |                             end_byte: 0,\n  504 |                         });\n  505 |                         class_docstring_attached = true;\n  506 |                     }\n  507 |                 }\n  508 |             }\n  509 |             _ => {\n  510 |                 if let Some(element) = reorderable_element {\n  511 |                     // If we haven't attached class docstring yet and this is the first real element,\n  512 |                     // create a separate docstring element (for cases where there's no extends)\n  513 |                     if !class_docstring_attached\n  514 |                         && !class_docstring_comments.is_empty()\n  515 |                         && !found_extends_declaration\n  516 |                     {\n  517 |                         let docstring_text = class_docstring_comments.join(\"\\n\");\n  518 |                         elements.push(GDScriptTokensWithComments {\n  519 |                             token_kind: GDScriptTokenKind::Docstring(docstring_text.clone()),\n  520 |                             attached_comments: Vec::new(),\n  521 |                             trailing_comments: Vec::new(),\n  522 |                             original_text: docstring_text,\n  523 |                             start_byte: 0,\n  524 |                             end_byte: 0,\n  525 |                         });\n  526 |                         class_docstring_attached = true;\n  527 |                     }\n  528 | \n  529 |                     let combined_comments =\n  530 |                         merge_pending_texts(&pending_annotations, &pending_comments);\n  531 | \n  532 |                     elements.push(GDScriptTokensWithComments {\n  533 |                         token_kind: element,\n  534 |                         attached_comments: combined_comments,\n  535 |                         trailing_comments: Vec::new(),\n  536 |                         original_text: text,\n  537 |                         start_byte: node.start_byte(),\n  538 |                         end_byte: node.end_byte(),\n  539 |                     });\n  540 |                     pending_comments.clear();\n  541 |                     pending_annotations.clear();\n  542 |                 } else {\n  543 |                     // We create unknown element for unhandled nodes to preserve\n  544 |                     // them. Given how the module works, if we don't do that the\n  545 |                     // nodes will be dropped.\n  546 |                     let combined_comments =\n  547 |                         merge_pending_texts(&pending_annotations, &pending_comments);\n  548 |                     elements.push(GDScriptTokensWithComments {\n  549 |                         token_kind: GDScriptTokenKind::Unknown(text.clone()),\n  550 |                         attached_comments: combined_comments,\n  551 |                         trailing_comments: Vec::new(),\n  552 |                         original_text: text,\n  553 |                         start_byte: node.start_byte(),\n  554 |                         end_byte: node.end_byte(),\n  555 |                     });\n  556 |                     pending_comments.clear();\n  557 |                     pending_annotations.clear();\n  558 |                 }\n  559 |             }\n  560 |         }\n  561 |     }\n  562 | \n  563 |     // `region_end_comment` stores a trailing `#endregion` that should follow\n  564 |     // the most recent function with a matching `#region`. If we found no\n  565 |     // matching function, we fall back to attaching it to the last element (or\n  566 |     // create a standalone element at the end). This avoids the formatter\n  567 |     // deleting or losing the directive when we reorder code blocks.\n  568 |     if let Some(region_end) = region_end_comment.take() {\n  569 |         if let Some(last_element) = elements.last_mut() {\n  570 |             last_element.trailing_comments.push(region_end);\n  571 |         } else {\n  572 |             elements.push(GDScriptTokensWithComments {\n  573 |                 token_kind: GDScriptTokenKind::Unknown(region_end.clone()),\n  574 |                 attached_comments: Vec::new(),\n  575 |                 trailing_comments: Vec::new(),\n  576 |                 original_text: region_end,\n  577 |                 start_byte: 0,\n  578 |                 end_byte: 0,\n  579 |             });\n  580 |         }\n  581 |     }\n  582 | \n  583 |     // Comments and annotations that we have not yet attached go at the end of\n  584 |     // the file. They are either trailing comments, stray `#region` markers, or\n  585 |     // unknown statements that appear after the last declaration (like new\n  586 |     // syntax in a dev version of Godot). We merge them in source order with\n  587 |     // `merge_pending_texts` and append them either to the last reordered\n  588 |     // element or to a dedicated \"Unknown\" token so the text remains in the\n  589 |     // output.\n  590 |     if !pending_annotations.is_empty() || !pending_comments.is_empty() {\n  591 |         let trailing_texts = merge_pending_texts(&pending_annotations, &pending_comments);\n  592 |         if let Some(last_element) = elements.last_mut() {\n  593 |             last_element.trailing_comments.extend(trailing_texts);\n  594 |         } else if !trailing_texts.is_empty() {\n  595 |             let combined_text = trailing_texts.join(\"\\n\");\n  596 |             elements.push(GDScriptTokensWithComments {\n  597 |                 token_kind: GDScriptTokenKind::Unknown(combined_text.clone()),\n  598 |                 attached_comments: Vec::new(),\n  599 |                 trailing_comments: Vec::new(),\n  600 |                 original_text: combined_text,\n  601 |                 start_byte: 0,\n  602 |                 end_byte: 0,\n  603 |             });\n  604 |         }\n  605 |     }\n  606 | \n  607 |     Ok(elements)\n  608 | }","lines":361}]},{"id":"ARCH_6","type":"architecture","type_label":"Architecture","severity":"negligible","easy":false,"description":"`render_indent` takes an `&IndentInfo` parameter but only uses the `indent.spaces` field; the `indent.column` field is ignored. The `IndentInfo` struct is passed solely to carry the `spaces` value here, making the API unnecessarily coupled to a two-field struct when a plain `usize` would suffice.","fix":"Change the signature to `fn render_indent(spaces: usize, indent_size: usize, use_spaces: bool) -> String` and update all three call sites to pass `indent.spaces` directly.","locations":[{"ref":"src/formatter.rs:1173-1184","code":" 1173 | fn render_indent(indent: &IndentInfo, indent_size: usize, use_spaces: bool) -> String {\n 1174 |     if use_spaces {\n 1175 |         return \" \".repeat(indent.spaces);\n 1176 |     }\n 1177 | \n 1178 |     let tabs = indent.spaces / indent_size;\n 1179 |     let remainder = indent.spaces % indent_size;\n 1180 |     let mut result = String::with_capacity(tabs + remainder);\n 1181 |     result.push_str(&\"\\t\".repeat(tabs));\n 1182 |     result.push_str(&\" \".repeat(remainder));\n 1183 |     result\n 1184 | }","lines":12}]},{"id":"OTHER_1","type":"other","type_label":"Other","severity":"medium","easy":false,"description":"`find_gdscript_files` uses a manual directory traversal stack without any cycle detection. On\nfilesystems with symlink loops (a directory symlinked to one of its own ancestors) the function will\nrecurse indefinitely, growing the stack until the process runs out of memory or stack space. This is\na real-world risk on Linux/macOS where symlinks are common in development environments.","fix":"Track visited canonical directory paths using a `HashSet<PathBuf>` of canonicalized paths:\n\n    use std::collections::HashSet;\n    let mut visited = HashSet::new();\n    // Before pushing a directory:\n    if let Ok(canonical) = entry.path().canonicalize() {\n        if visited.insert(canonical) {\n            paths_to_check.push(entry.path());\n        }\n    }","locations":[{"ref":"src/main.rs:357-383","code":"  357 |     while let Some(current_path) = paths_to_check.pop() {\n  358 |         if current_path.is_dir() {\n  359 |             let entries = fs::read_dir(&current_path).map_err(|error| {\n  360 |                 format!(\n  361 |                     \"Failed to read directory {}: {}\",\n  362 |                     current_path.display(),\n  363 |                     error\n  364 |                 )\n  365 |             })?;\n  366 |             for entry in entries {\n  367 |                 let entry = entry.map_err(|error| {\n  368 |                     format!(\n  369 |                         \"Failed to read entry in {}: {}\",\n  370 |                         current_path.display(),\n  371 |                         error\n  372 |                     )\n  373 |                 })?;\n  374 |                 if entry.path().is_dir() {\n  375 |                     paths_to_check.push(entry.path());\n  376 |                 } else if entry.path().extension().is_some_and(|ext| ext == \"gd\") {\n  377 |                     gdscript_file_paths.push(entry.path());\n  378 |                 }\n  379 |             }\n  380 |         } else if current_path.extension().is_some_and(|ext| ext == \"gd\") {\n  381 |             gdscript_file_paths.push(current_path);\n  382 |         }\n  383 |     }","lines":27}]}]</script>
<script type="application/json" id="PI">{"name":"GDScript-formatter","description":"","git_url":"https://github.com/GDQuest/GDScript-formatter.git","repo_url":"https://github.com/GDQuest/GDScript-formatter","commit":"039d405ed246a34a7e456b0894c0b9216efc8ff7","commit_short":"039d405ed2","commit_url":"https://github.com/GDQuest/GDScript-formatter/commit/039d405ed246a34a7e456b0894c0b9216efc8ff7","branch":"main","generated":"2026-06-16"}</script>
<div class="app">
<div class="top">
  <div>
    <h1>Code Audit Report &mdash; GDScript-formatter</h1>
    <div class="meta">Generated 2026-06-16 &mdash; <span id="stats"></span></div>
  </div>
  <div class="top-right">
    <input type="search" id="search" placeholder="Search findings…" autocomplete="off">
    <label class="group-label">Group by
      <select id="groupby">
        <option value="type">Type</option>
        <option value="severity">Severity</option>
        <option value="file">File</option>
        <option value="none">None (flat)</option>
      </select>
    </label>
  </div>
</div>
<div class="project-info"><div class="pi-name">GDScript-formatter</div><div class="pi-row"><a class="pi-repo-btn" href="https://github.com/GDQuest/GDScript-formatter" target="_blank" rel="noopener">&#128279; https://github.com/GDQuest/GDScript-formatter</a><span><span class="pi-k">branch</span> <code>main</code></span><span><span class="pi-k">commit</span> <a href="https://github.com/GDQuest/GDScript-formatter/commit/039d405ed246a34a7e456b0894c0b9216efc8ff7" target="_blank" rel="noopener"><code>039d405ed2</code></a></span></div></div>
<div class="filterbar" id="filterbar"></div>
<div id="progress-wrap">
  <div id="progress-bar"><div id="progress-fill"></div></div>
  <span id="progress-text"></span>
  <button class="action-btn" data-action="collapse-all">⊟ Collapse all</button>
  <button class="action-btn" data-action="expand-all">⊞ Expand all</button>
  <button class="action-btn" data-action="download-md">&#11015; MD</button>
  <button class="action-btn" data-action="download-md-short">&#11015; MD Short</button>
  <button class="action-btn" data-action="download-toml">&#11015; TOML</button>
</div>
<div id="hidden-bar"></div>
<div id="content"></div>
</div>
<script>
(function(){
'use strict';

// Data
const FINDINGS     = JSON.parse(document.getElementById('D').textContent);
const PROJECT_INFO = JSON.parse(document.getElementById('PI').textContent);
const SEVS     = ['critical','high','medium','low','negligible'];
const STATUSES = ['open','fixed','wont-fix','false-positive'];
const ALL_TYPES = [...new Set(FINDINGS.map(f => f.type))].sort();
// Build lookup map for O(1) access
const FINDING_BY_ID = new Map(FINDINGS.map(f => [f.id, f]));
// Only severities actually present in the report — the filter bar hides the rest.
const PRESENT_SEVS = SEVS.filter(s => FINDINGS.some(f => (f.severity || 'medium') === s));
// Whether any finding is flagged easy — the "Quick wins" filter row is hidden otherwise.
const HAS_EASY = FINDINGS.some(f => f.easy);
const STORE_KEY = 'audit-status:' + location.href;

const STATUS_CLS = {open:'st-open', fixed:'st-fixed', 'wont-fix':'st-wont', 'false-positive':'st-fp'};
const STATUS_LBL = {open:'● open', fixed:'✓ fixed', 'wont-fix':"— won't fix", 'false-positive':'✗ false pos.'};
const STATUS_NEXT = {open:'fixed', fixed:'wont-fix', 'wont-fix':'false-positive', 'false-positive':'open'};

// State
const S = {
  search: '', groupBy: 'type',
  hiddenSev: new Set(), hiddenType: new Set(),
  hiddenStatus: new Set(), hiddenGroup: new Set(),
  collapsed: new Set(), easyOnly: false,
};
const statuses = {};

// Persistence
function loadStatuses() {
  try { Object.assign(statuses, JSON.parse(localStorage.getItem(STORE_KEY) || '{}')); } catch {}
}
function saveStatuses() {
  try { localStorage.setItem(STORE_KEY, JSON.stringify(statuses)); } catch {}
}
function statusOf(id) { return statuses[id] || 'open'; }

// URL hash — debounced so rapid filter changes don't thrash history
let _hashTimer = 0;
function syncHash() {
  clearTimeout(_hashTimer);
  _hashTimer = setTimeout(() => {
    const p = new URLSearchParams();
    if (S.groupBy !== 'type') p.set('g', S.groupBy);
    if (S.hiddenSev.size)    p.set('hs',  [...S.hiddenSev].join(','));
    if (S.hiddenType.size)   p.set('ht',  [...S.hiddenType].join(','));
    if (S.hiddenStatus.size) p.set('hst', [...S.hiddenStatus].join(','));
    if (S.easyOnly)          p.set('e',   '1');
    if (S.search)            p.set('q',   S.search);
    const str = p.toString();
    history.replaceState(null, '', str ? '#' + str : location.pathname + location.search);
  }, 300);
}

function loadHash() {
  const p = new URLSearchParams(location.hash.slice(1));
  if (p.has('g'))   { S.groupBy = p.get('g'); document.getElementById('groupby').value = S.groupBy; }
  if (p.has('q'))   { S.search  = p.get('q'); document.getElementById('search').value  = S.search;  }
  if (p.has('hs'))  p.get('hs') .split(',').filter(Boolean).forEach(v => S.hiddenSev.add(v));
  if (p.has('ht'))  p.get('ht') .split(',').filter(Boolean).forEach(v => S.hiddenType.add(v));
  if (p.has('hst')) p.get('hst').split(',').filter(Boolean).forEach(v => S.hiddenStatus.add(v));
  if (p.has('e'))   S.easyOnly = p.get('e') === '1';
}

// Helpers
function esc(s) {
  return String(s).replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;').replace(/"/g,'&quot;');
}

function highlight(text, q) {
  if (!q) return esc(text);
  const re = new RegExp('(' + q.replace(/[.*+?^${}()|[\]\\]/g,'\\$&') + ')', 'gi');
  return String(text).split(re).map((p, i) => i % 2 ? '<mark>' + esc(p) + '</mark>' : esc(p)).join('');
}

// Render text with ```fenced``` code blocks and `inline` backticks,
// also paragraph-splitting on blank lines. Output is HTML-safe.
function renderMd(text, q) {
  const src = String(text == null ? '' : text);
  const FENCE = /```[ \t]*([A-Za-z0-9_+-]*)[ \t]*\r?\n([\s\S]*?)```/g;
  const parts = [];
  let last = 0, m;
  while ((m = FENCE.exec(src)) !== null) {
    parts.push({kind: 'text', value: src.slice(last, m.index)});
    parts.push({kind: 'code', value: m[2].replace(/\r?\n$/, '')});
    last = m.index + m[0].length;
  }
  parts.push({kind: 'text', value: src.slice(last)});

  function renderInline(s) {
    const re = /`([^`\n]+)`/g;
    let out = '', i = 0, mm;
    while ((mm = re.exec(s)) !== null) {
      out += highlight(s.slice(i, mm.index), q);
      out += '<code class="md-inline">' + highlight(mm[1], q) + '</code>';
      i = mm.index + mm[0].length;
    }
    out += highlight(s.slice(i), q);
    return out;
  }

  function renderTextChunk(s) {
    const paragraphs = s.split(/\r?\n[ \t]*\r?\n/);
    const out = [];
    for (const p of paragraphs) {
      const trimmed = p.replace(/^\s+|\s+$/g, '');
      if (!trimmed) continue;
      out.push('<p>' + renderInline(trimmed).replace(/\r?\n/g, '<br>') + '</p>');
    }
    return out.join('');
  }

  let html = '';
  for (const p of parts) {
    if (p.kind === 'text') html += renderTextChunk(p.value);
    else html += '<pre class="md-code">' + highlight(p.value, q) + '</pre>';
  }
  return html;
}

function sevCls(s)    { return SEVS.includes(s) ? 'sev-'+s : 'sev-negligible'; }
function sevLbl(s)    { return s ? s[0].toUpperCase()+s.slice(1) : 'Medium'; }
function typeLabel(t) { return t.replace(/_/g,' ').replace(/\b\w/g, c => c.toUpperCase()); }

// First sentence of text (up to and including the first period).
function firstSentence(text) {
  const m = String(text).match(/\.(\s|$)/);
  return m ? text.slice(0, text.indexOf(m[0]) + 1) : text;
}

// Build full MD report (mirrors to_markdown in Python).
function buildMdFull() {
  const SORD = ['critical','high','medium','low','negligible'];
  const today = new Date().toISOString().slice(0, 10);
  let out = '# Code Audit Report\n\n_Generated ' + today + '_\n\n';

  const typeMap = new Map();
  for (const f of FINDINGS) {
    if (!typeMap.has(f.type)) typeMap.set(f.type, []);
    typeMap.get(f.type).push(f);
  }
  const sortedTypes = [...typeMap.keys()].sort();

  out += '## Summary\n\n| Type | Severity | Count |\n|------|----------|-------|\n';
  for (const t of sortedTypes) {
    const items = typeMap.get(t);
    const topSev = items.map(f => f.severity || 'medium').reduce((best, s) => {
      const bi = SORD.indexOf(best), si = SORD.indexOf(s);
      return (si >= 0 && (bi < 0 || si < bi)) ? s : best;
    }, 'negligible');
    out += '| ' + typeLabel(t) + ' | ' + topSev + ' | ' + items.length + ' |\n';
  }

  for (const t of sortedTypes) {
    out += '\n## ' + typeLabel(t) + '\n\n';
    for (const f of typeMap.get(t)) {
      const sevStr  = f.severity ? ' `' + f.severity.toUpperCase() + '`' : '';
      const easyStr = f.easy ? ' ⚡ easy' : '';
      out += '### ' + f.id + sevStr + easyStr + '\n\n';
      out += '**Description:** ' + (f.description || '') + '\n\n';
      if (f.locations.length) {
        out += '**Locations:**\n\n';
        for (const l of f.locations) {
          if (l.code != null) out += '```\n' + l.ref + '\n' + l.code + '\n```\n\n';
          else                out += '- `' + l.ref + '` _(file not found)_\n\n';
        }
      }
      out += '**Fix:** ' + (f.fix || '') + '\n\n---\n\n';
    }
  }
  return out;
}

// Build MD short checklist (mirrors to_md_short in Python).
function buildMdShort() {
  const lines = [];
  const ids   = [];
  const sorted = [...FINDINGS].sort((a, b) => a.type.localeCompare(b.type));
  for (const f of sorted) {
    ids.push(f.id);
    const desc    = firstSentence(f.description || '');
    const locs    = f.locations.map(l => l.ref);
    const locStr  = locs.length ? ' - [' + locs.join(', ') + ']' : '';
    const sevStr  = f.severity ? ' [' + f.severity.toUpperCase() + ']' : '';
    const easyStr = f.easy ? ' ⚡' : '';
    lines.push('- [ ] - ' + f.id + sevStr + easyStr + ' - ' + desc + locStr);
  }
  lines.push('', '## IDs only', '');
  ids.forEach(id => lines.push('- [ ] - ' + id));
  return lines.join('\n') + '\n';
}

function downloadText(text, filename, mime) {
  const a = document.createElement('a');
  a.href = URL.createObjectURL(new Blob([text], {type: mime || 'text/plain'}));
  a.download = filename;
  a.click();
  setTimeout(() => URL.revokeObjectURL(a.href), 1000);
}

function tomlStr(s) {
  return '"' + String(s == null ? '' : s)
    .replace(/\\/g, '\\\\')
    .replace(/"/g, '\\"')
    .replace(/\n/g, '\\n')
    .replace(/\r/g, '\\r')
    .replace(/\t/g, '\\t') + '"';
}

// Build TOML matching the original findings.toml format (re-parseable by toml_to_report.py).
function buildFindingsToml() {
  function pad(key) { return (key + '            ').slice(0, 12); }
  const chunks = [];
  for (const f of FINDINGS) {
    const lns = ['[[finding]]'];
    lns.push(pad('type')        + '= ' + tomlStr(f.type));
    lns.push(pad('severity')    + '= ' + tomlStr(f.severity || 'medium'));
    if (f.easy) lns.push(pad('easy') + '= true');
    lns.push(pad('description') + '= ' + tomlStr(f.description || ''));
    const refs = f.locations.map(l => tomlStr(l.ref));
    lns.push(pad('locations')   + '= [' + refs.join(', ') + ']');
    lns.push(pad('fix')         + '= ' + tomlStr(f.fix || ''));
    chunks.push(lns.join('\n'));
  }
  return chunks.join('\n\n') + '\n';
}

// Build project-info.toml from embedded PROJECT_INFO (same format as gen_project_info.py).
function buildProjectInfoToml() {
  const ORDER = ['name','description','git_url','repo_url','commit','commit_short','commit_url','branch','generated'];
  const pi = Object.assign({}, PROJECT_INFO);
  if (!('description' in pi)) pi.description = '';
  const lines = [
    '# Project metadata for the code audit report (all fields optional).',
    '# Hand-edit freely (e.g. fill in description); re-run the audit without --force to keep edits.',
    '', '[project]'
  ];
  for (const k of ORDER) {
    if (k in pi) lines.push(k + ' = ' + tomlStr(pi[k]));
  }
  return lines.join('\n') + '\n';
}

// Build the GitHub-ready Markdown for one finding (mirrors findings.md).
function findingToMd(f) {
  let out = '### ' + f.id + (f.severity ? ' `' + f.severity.toUpperCase() + '`' : '') + (f.easy ? ' ⚡ easy' : '') + '\n\n';
  out += '**Description:** ' + (f.description || '') + '\n\n';
  if (f.locations.length) {
    out += '**Locations:**\n\n';
    for (const l of f.locations) {
      if (l.code != null) out += '```\n' + l.ref + '\n' + l.code + '\n```\n\n';
      else                out += '- `' + l.ref + '` _(file not found)_\n\n';
    }
  }
  out += '**Fix:** ' + (f.fix || '') + '\n';
  return out;
}

function flashBtn(btn, msg) {
  btn.textContent = msg;
  setTimeout(() => { btn.textContent = '⧉ Copy'; }, 1200);
}

function copyText(text, btn) {
  const ok = () => flashBtn(btn, '✓ Copied');
  const no = () => flashBtn(btn, '✗ Failed');
  const fallback = () => {
    try {
      const ta = document.createElement('textarea');
      ta.value = text; ta.style.position = 'fixed'; ta.style.top = '-9999px';
      document.body.appendChild(ta); ta.select();
      const r = document.execCommand('copy');
      document.body.removeChild(ta);
      r ? ok() : no();
    } catch { no(); }
  };
  if (navigator.clipboard && navigator.clipboard.writeText)
    navigator.clipboard.writeText(text).then(ok, fallback);
  else fallback();
}

// Filter logic
function getKey(f) {
  if (S.groupBy === 'severity') return f.severity || 'medium';
  if (S.groupBy === 'file')     return f.locations.length ? f.locations[0].ref.replace(/:.*/,'') : '(no location)';
  if (S.groupBy === 'none')     return '';
  return f.type_label;
}

// Pre-compute search haystack per finding (cleared on full re-render)
const _hay = new Map();
function hayOf(f) {
  if (!_hay.has(f.id)) {
    _hay.set(f.id, [f.id, f.type_label, f.severity, f.description, f.fix,
                    f.easy ? 'easy quick win' : '',
                    ...f.locations.map(l => l.ref)].join(' ').toLowerCase());
  }
  return _hay.get(f.id);
}

function matches(f) {
  if (S.easyOnly && !f.easy)                   return false;
  if (S.hiddenSev.has(f.severity || 'medium')) return false;
  if (S.hiddenType.has(f.type))                return false;
  const st = statusOf(f.id);
  if (S.hiddenStatus.has(st))                  return false;
  if (S.search) {
    const q = S.search.toLowerCase();
    if (!hayOf(f).includes(q) && !st.includes(q)) return false;
  }
  return true;
}

function sortKeys(keys) {
  if (S.groupBy === 'severity') {
    return [...keys].sort((a, b) => {
      const ai = SEVS.indexOf(a), bi = SEVS.indexOf(b);
      return (ai < 0 ? 99 : ai) - (bi < 0 ? 99 : bi);
    });
  }
  return [...keys].sort();
}

// Build body HTML for an expanded card (called lazily on first expand).
function renderCardBody(f, q) {
  let locs = '';
  for (const l of f.locations) {
    const lineInfo = l.lines != null
      ? ' <span class="loc-lines">(' + l.lines + ' line' + (l.lines === 1 ? '' : 's') + ')</span>'
      : '';
    locs += '<div class="loc-block"><div class="loc-ref">' + highlight(l.ref, q) + lineInfo + '</div>';
    locs += l.code != null ? '<pre>' + esc(l.code) + '</pre>' : '<div class="not-found">(file not found)</div>';
    locs += '</div>';
  }
  return (
    '<div class="field-lbl">Description</div>' +
    '<div class="desc">'    + renderMd(f.description, q) + '</div>' +
    (locs ? '<div class="field-lbl">Locations</div>' + locs : '') +
    '<div class="field-lbl">Fix</div>' +
    '<div class="fix-txt">' + renderMd(f.fix, q) + '</div>'
  );
}

// Render card shell only — header + empty or populated body.
// When collapsed the body is empty (display:none) — avoids renderMd for 1800+ cards.
function renderCard(f, q) {
  const sc  = sevCls(f.severity || 'medium');
  const sl  = sevLbl(f.severity || 'medium');
  const st  = statusOf(f.id);
  const sc2 = STATUS_CLS[st] || 'st-open';
  const sl2 = STATUS_LBL[st] || '● open';
  const isCollapsed = S.collapsed.has(f.id);
  const isResolved  = st !== 'open';

  const locSummary = f.locations.length
    ? f.locations[0].ref + (f.locations.length > 1 ? ' \u2026' : '')
    : '';

  const bodyInner = isCollapsed ? '' : renderCardBody(f, q);

  return (
    '<div class="card' + (isResolved ? ' resolved' : '') + '" data-card-id="' + esc(f.id) + '">' +
    '<div class="card-header" data-action="toggle-card" data-id="' + esc(f.id) + '">' +
    '<span class="card-arrow">' + (isCollapsed ? '\u25b6' : '\u25bc') + '</span>' +
    '<span class="card-id">' + esc(f.id) + '</span>' +
    '<span class="badge ' + sc + '" style="cursor:default">' + esc(sl) + '</span>' +
    '<span class="badge type-badge" style="cursor:default">' + esc(f.type_label) + '</span>' +
    (f.easy ? '<span class="badge easy-badge" style="cursor:default" title="Easy to verify &amp; fix - no deep knowledge of the code, SDK or technology required">\u26a1 Easy</span>' : '') +
    (locSummary ? '<span class="card-loc" title="' + esc(locSummary) + '">' + esc(locSummary) + '</span>' : '') +
    '<button class="status-btn ' + sc2 + '" data-action="cycle-status" data-id="' + esc(f.id) + '">' + sl2 + '</button>' +
    '<button class="copy-btn" data-action="copy-md" data-id="' + esc(f.id) + '" title="Copy as Markdown for GitHub">\u29c9 Copy</button>' +
    '</div>' +
    '<div class="card-body" data-body-id="' + esc(f.id) + '"' + (isCollapsed ? ' style="display:none"' : '') + '>' +
    bodyInner +
    '</div></div>'
  );
}

function initFilterBar() {
  const fb = document.getElementById('filterbar');
  let h = '<div class="filter-row"><span class="filter-lbl">Severity:</span>';
  for (const s of PRESENT_SEVS)
    h += '<button class="badge ' + sevCls(s) + '" data-sev="' + s + '">' + sevLbl(s) + '</button>';
  h += '<button class="filter-toggle" data-toggle-all="sev"></button></div>';

  h += '<div class="filter-row"><span class="filter-lbl">Type:</span>';
  for (const t of ALL_TYPES)
    h += '<button class="badge type-badge" data-type="' + esc(t) + '">' + esc(typeLabel(t)) + '</button>';
  h += '<button class="filter-toggle" data-toggle-all="type"></button></div>';

  h += '<div class="filter-row"><span class="filter-lbl">Status:</span>';
  for (const st of STATUSES)
    h += '<button class="badge ' + (STATUS_CLS[st]||'st-open') + '" data-status="' + st + '">' + (STATUS_LBL[st]||st) + '</button>';
  h += '<button class="filter-toggle" data-toggle-all="status"></button></div>';

  if (HAS_EASY)
    h += '<div class="filter-row"><span class="filter-lbl">Quick wins:</span>'
       + '<button class="badge easy-badge" data-easy="1">\u26a1 Easy only</button></div>';

  fb.innerHTML = h;
}

function updateToggles() {
  for (const b of document.querySelectorAll('[data-sev]'))    b.classList.toggle('off', S.hiddenSev.has(b.dataset.sev));
  for (const b of document.querySelectorAll('[data-type]'))   b.classList.toggle('off', S.hiddenType.has(b.dataset.type));
  for (const b of document.querySelectorAll('[data-status]')) b.classList.toggle('off', S.hiddenStatus.has(b.dataset.status));
  for (const b of document.querySelectorAll('[data-easy]'))   b.classList.toggle('off', !S.easyOnly);
  for (const b of document.querySelectorAll('[data-toggle-all]')) {
    const k = b.dataset.toggleAll;
    const set = k === 'sev' ? S.hiddenSev : k === 'type' ? S.hiddenType : S.hiddenStatus;
    b.textContent = set.size === 0 ? 'Hide all' : 'Show all';
  }
}

function updateProgress() {
  const total = FINDINGS.length;
  const done  = FINDINGS.filter(f => statusOf(f.id) !== 'open').length;
  const pct   = total ? Math.round(done / total * 100) : 0;
  document.getElementById('progress-fill').style.width = pct + '%';
  document.getElementById('progress-text').textContent = done + ' / ' + total + ' resolved (' + pct + '%)';
}

// Surgical toggle: flip one card without touching the rest of the DOM.
function toggleCard(id) {
  const wasCollapsed = S.collapsed.has(id);
  if (wasCollapsed) S.collapsed.delete(id); else S.collapsed.add(id);
  const body  = document.querySelector('[data-body-id="' + id + '"]');
  const arrow = document.querySelector('[data-card-id="' + id + '"] .card-arrow');
  if (!body) { render(); return; }
  if (wasCollapsed) {
    if (!body.firstChild) {
      const f = FINDING_BY_ID.get(id);
      if (f) body.innerHTML = renderCardBody(f, S.search);
    }
    body.style.display = '';
    if (arrow) arrow.textContent = '\u25bc';
  } else {
    body.style.display = 'none';
    if (arrow) arrow.textContent = '\u25b6';
  }
}

// Surgical status update: update one card's button + resolved class.
function cycleStatus(id) {
  statuses[id] = STATUS_NEXT[statusOf(id)] || 'open';
  saveStatuses();
  const st  = statusOf(id);
  const btn = document.querySelector('.status-btn[data-id="' + id + '"]');
  if (!btn) { render(); return; }
  btn.className   = 'status-btn ' + (STATUS_CLS[st] || 'st-open');
  btn.textContent = STATUS_LBL[st] || '\u25cf open';
  const card = document.querySelector('[data-card-id="' + id + '"]');
  if (card) card.classList.toggle('resolved', st !== 'open');
  updateProgress();
}

// Surgical collapse/expand for a group: toggle all bodies in the group without full re-render.
function collapseGroup(key, collapse) {
  FINDINGS.forEach(f => {
    if (getKey(f) !== key) return;
    if (collapse) S.collapsed.add(f.id); else S.collapsed.delete(f.id);
    const body  = document.querySelector('[data-body-id="' + f.id + '"]');
    const arrow = document.querySelector('[data-card-id="' + f.id + '"] .card-arrow');
    if (!body) return;
    if (collapse) {
      body.style.display = 'none';
      if (arrow) arrow.textContent = '\u25b6';
    } else {
      if (!body.firstChild) {
        const ff = FINDING_BY_ID.get(f.id);
        if (ff) body.innerHTML = renderCardBody(ff, S.search);
      }
      body.style.display = '';
      if (arrow) arrow.textContent = '\u25bc';
    }
  });
}

// Full re-render: used for filter/group-by/search changes.
// Batched via requestAnimationFrame.
let _rafPending = false;
function scheduleRender() {
  if (_rafPending) return;
  _rafPending = true;
  requestAnimationFrame(() => { _rafPending = false; render(); });
}

function render() {
  _hay.clear();
  updateToggles();
  updateProgress();

  const q       = S.search;
  const visible = FINDINGS.filter(matches);
  document.getElementById('stats').textContent = visible.length + ' / ' + FINDINGS.length + ' findings';

  // Build groups
  const map = new Map();
  for (const f of visible) {
    const k = getKey(f);
    if (!map.has(k)) map.set(k, []);
    map.get(k).push(f);
  }

  // Hidden-groups bar
  const hbar = document.getElementById('hidden-bar');
  if (S.hiddenGroup.size > 0) {
    let hh = '<div class="hidden-inner"><span class="hidden-lbl">Hidden:</span>';
    for (const g of [...S.hiddenGroup].sort())
      hh += '<button class="show-btn" data-action="show-group" data-key="' + esc(g) + '">' + esc(g || '(all findings)') + '</button>';
    hh += '<button class="show-all-btn" data-action="show-all">Show all</button></div>';
    hbar.innerHTML = hh;
  } else {
    hbar.innerHTML = '';
  }

  // Content
  const content = document.getElementById('content');
  if (!visible.length) {
    content.innerHTML = '<div class="no-results">No findings match the current filters.</div>';
    syncHash();
    return;
  }

  let hh = '';
  for (const k of sortKeys([...map.keys()])) {
    if (S.hiddenGroup.has(k)) continue;
    const items   = map.get(k);
    const title   = k || 'All Findings';
    const done    = items.filter(f => statusOf(f.id) !== 'open').length;
    const allDone = done === items.length;
    hh += '<div class="group-section">' +
      '<div class="group-header">' +
      '<span class="group-title' + (allDone ? ' all-done' : '') + '">' + esc(title) + '</span>' +
      '<span class="group-count">' + done + '/' + items.length + '</span>' +
      '<button class="icon-btn" data-action="collapse-group" data-key="' + esc(k) + '" title="Collapse all">\u229f</button>' +
      '<button class="icon-btn" data-action="expand-group"   data-key="' + esc(k) + '" title="Expand all">\u229e</button>' +
      '<button class="hide-btn" data-action="hide-group"     data-key="' + esc(k) + '">Hide</button>' +
      '</div>';
    for (const f of items) hh += renderCard(f, q);
    hh += '</div>';
  }
  content.innerHTML = hh;
  syncHash();
}

// Event delegation
document.getElementById('filterbar').addEventListener('click', e => {
  const b = e.target.closest('[data-sev],[data-type],[data-status],[data-toggle-all],[data-easy]');
  if (!b) return;
  if (b.dataset.easy) {
    S.easyOnly = !S.easyOnly;
  }
  else if (b.dataset.toggleAll) {
    const k = b.dataset.toggleAll;
    const set  = k === 'sev' ? S.hiddenSev : k === 'type' ? S.hiddenType : S.hiddenStatus;
    const full = k === 'sev' ? PRESENT_SEVS : k === 'type' ? ALL_TYPES    : STATUSES;
    if (set.size === 0) full.forEach(v => set.add(v));
    else                set.clear();
  }
  else if (b.dataset.sev)    { const s = S.hiddenSev;    s.has(b.dataset.sev)    ? s.delete(b.dataset.sev)    : s.add(b.dataset.sev); }
  else if (b.dataset.type)   { const s = S.hiddenType;   s.has(b.dataset.type)   ? s.delete(b.dataset.type)   : s.add(b.dataset.type); }
  else if (b.dataset.status) { const s = S.hiddenStatus; s.has(b.dataset.status) ? s.delete(b.dataset.status) : s.add(b.dataset.status); }
  scheduleRender();
});

document.addEventListener('click', e => {
  const b = e.target.closest('[data-action]');
  if (!b) return;
  const { action, key, id } = b.dataset;

  if      (action === 'hide-group')     { S.hiddenGroup.add(key);    scheduleRender(); }
  else if (action === 'show-group')     { S.hiddenGroup.delete(key); scheduleRender(); }
  else if (action === 'show-all')       { S.hiddenGroup.clear();     scheduleRender(); }
  else if (action === 'collapse-all')   { FINDINGS.forEach(f => S.collapsed.add(f.id)); scheduleRender(); }
  else if (action === 'expand-all')     { S.collapsed.clear(); scheduleRender(); }
  else if (action === 'copy-md') {
    e.stopPropagation();
    const f = FINDING_BY_ID.get(id);
    if (f) copyText(findingToMd(f), b);
  }
  else if (action === 'download-md')       { downloadText(buildMdFull(),  'findings.md'); }
  else if (action === 'download-md-short') { downloadText(buildMdShort(), 'findings-short.md'); }
  else if (action === 'download-toml') {
    downloadText(buildFindingsToml(),    'findings.toml',     'application/toml');
    downloadText(buildProjectInfoToml(), 'project-info.toml', 'application/toml');
  }
  else if (action === 'cycle-status') {
    e.stopPropagation();
    cycleStatus(id);
  }
  else if (action === 'toggle-card') {
    toggleCard(id);
  }
  else if (action === 'collapse-group') {
    collapseGroup(key, true);
  }
  else if (action === 'expand-group') {
    collapseGroup(key, false);
  }
});

// Debounced search — 180 ms after last keystroke
let _searchTimer = 0;
document.getElementById('search').addEventListener('input', e => {
  S.search = e.target.value;
  clearTimeout(_searchTimer);
  _searchTimer = setTimeout(scheduleRender, 180);
});
document.getElementById('groupby').addEventListener('change', e => {
  S.groupBy = e.target.value;
  S.hiddenGroup.clear();
  scheduleRender();
});

// Init
loadStatuses();
initFilterBar();
loadHash();
FINDINGS.forEach(f => S.collapsed.add(f.id));
render();
})();
</script>
</body></html>