feat(frontend-check): flag raw <button> in JSX — use the shared <Button> component
New rules no-raw-button / no-raw-button-in-plugin ride inside Check 5's walkers, same as the browser-confirm rules. Raw buttons bypass the action=/entity= automation attributes (docs/BUTTON_AUTOMATION.md), so MCP/Puppeteer can't target them. Scope: .tsx/.jsx only (plain .ts builds DOM strings for non-React surfaces), components/ui/ exempt (defines the primitives — same carve-out as the button-automation check). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
fbe14f6c57
commit
c3bb958f23
37
frontend.go
37
frontend.go
@ -92,6 +92,11 @@ var (
|
|||||||
// Anti-pattern: browser confirm() instead of the shared confirm dialog
|
// Anti-pattern: browser confirm() instead of the shared confirm dialog
|
||||||
reWindowConfirm = regexp.MustCompile(`\bwindow\.confirm\s*\(`)
|
reWindowConfirm = regexp.MustCompile(`\bwindow\.confirm\s*\(`)
|
||||||
reBareConfirm = regexp.MustCompile(`(?:^|[^\w.])confirm\s*\(`)
|
reBareConfirm = regexp.MustCompile(`(?:^|[^\w.])confirm\s*\(`)
|
||||||
|
|
||||||
|
// Anti-pattern: raw <button> in JSX instead of the shared <Button> component.
|
||||||
|
// Raw buttons bypass the action=/entity= automation attributes
|
||||||
|
// (docs/BUTTON_AUTOMATION.md), so MCP/Puppeteer can't target them.
|
||||||
|
reRawButton = regexp.MustCompile(`<button\b`)
|
||||||
)
|
)
|
||||||
|
|
||||||
func checkFrontend(webSrcDir string) (violations []frontendViolation, warnings []frontendViolation) {
|
func checkFrontend(webSrcDir string) (violations []frontendViolation, warnings []frontendViolation) {
|
||||||
@ -131,6 +136,13 @@ func checkFrontend(webSrcDir string) (violations []frontendViolation, warnings [
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Raw <button> only applies to JSX files; plain .ts/.js build DOM via
|
||||||
|
// HTML strings for non-React surfaces (player, editor iframe) where the
|
||||||
|
// <Button> component doesn't exist. components/ui/ defines the
|
||||||
|
// primitives, so it is exempt (same carve-out as the automation check).
|
||||||
|
checkRawButton := (ext == ".tsx" || ext == ".jsx") &&
|
||||||
|
!strings.HasPrefix(filepath.ToSlash(relPath), "components/ui/")
|
||||||
|
|
||||||
lines, err := readSourceLines(path)
|
lines, err := readSourceLines(path)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil
|
return nil
|
||||||
@ -174,6 +186,13 @@ func checkFrontend(webSrcDir string) (violations []frontendViolation, warnings [
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if checkRawButton && isRawButtonJSXLine(line) {
|
||||||
|
violations = append(violations, frontendViolation{
|
||||||
|
file: relPath, line: lineNum, rule: "no-raw-button",
|
||||||
|
snippet: strings.TrimSpace(line),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// For allowed files, skip the remaining checks
|
// For allowed files, skip the remaining checks
|
||||||
if isAllowed {
|
if isAllowed {
|
||||||
continue
|
continue
|
||||||
@ -320,6 +339,14 @@ func checkPluginPages(dirs []string) []frontendViolation {
|
|||||||
snippet: strings.TrimSpace(line),
|
snippet: strings.TrimSpace(line),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ext == ".tsx" && !strings.HasPrefix(filepath.ToSlash(relPath), "components/ui/") &&
|
||||||
|
isRawButtonJSXLine(line) {
|
||||||
|
violations = append(violations, frontendViolation{
|
||||||
|
file: relPath, line: lineNum, rule: "no-raw-button-in-plugin",
|
||||||
|
snippet: strings.TrimSpace(line),
|
||||||
|
})
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
@ -359,6 +386,16 @@ func isCommentLine(trimmed string) bool {
|
|||||||
return strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "*") || strings.HasPrefix(trimmed, "/*")
|
return strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "*") || strings.HasPrefix(trimmed, "/*")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// isRawButtonJSXLine reports whether the line opens a raw HTML <button>
|
||||||
|
// element. JSX comment lines ({/* ... */}) are ignored here; the walkers
|
||||||
|
// already filter //-style comment lines via isCommentLine.
|
||||||
|
func isRawButtonJSXLine(line string) bool {
|
||||||
|
if strings.HasPrefix(strings.TrimSpace(line), "{/*") {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return reRawButton.MatchString(line)
|
||||||
|
}
|
||||||
|
|
||||||
func isBrowserConfirmCall(lines []string, idx int) bool {
|
func isBrowserConfirmCall(lines []string, idx int) bool {
|
||||||
line := lines[idx]
|
line := lines[idx]
|
||||||
if reWindowConfirm.MatchString(line) {
|
if reWindowConfirm.MatchString(line) {
|
||||||
|
|||||||
105
frontend_test.go
105
frontend_test.go
@ -96,6 +96,111 @@ func TestIsBrowserConfirmCall(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestIsRawButtonJSXLine(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
line string
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{line: `<button>`, want: true},
|
||||||
|
{line: ` <button`, want: true},
|
||||||
|
{line: `<button className="h-8 w-8" onClick={onPick}>`, want: true},
|
||||||
|
{line: `return <button type="button">{label}</button>`, want: true},
|
||||||
|
{line: `<Button action="save">Save</Button>`, want: false},
|
||||||
|
{line: `</button>`, want: false},
|
||||||
|
{line: `{/* <button> is banned in admin UI */}`, want: false},
|
||||||
|
{line: `<buttonish-element>`, want: false},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
if got := isRawButtonJSXLine(tt.line); got != tt.want {
|
||||||
|
t.Fatalf("isRawButtonJSXLine(%q) = %v, want %v", tt.line, got, tt.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCheckFrontendFlagsRawButton(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
if err := os.MkdirAll(filepath.Join(dir, "components", "editor"), 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, "components", "editor", "toolbar.tsx"), []byte(`export function Toolbar() {
|
||||||
|
return (
|
||||||
|
<div>
|
||||||
|
<button onClick={save}>Save</button>
|
||||||
|
<Button action="cancel">Cancel</Button>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
`), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
violations, _ := checkFrontend(dir)
|
||||||
|
var rawButton []frontendViolation
|
||||||
|
for _, v := range violations {
|
||||||
|
if v.rule == "no-raw-button" {
|
||||||
|
rawButton = append(rawButton, v)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(rawButton) != 1 {
|
||||||
|
t.Fatalf("expected exactly one no-raw-button violation, got %d: %#v", len(rawButton), violations)
|
||||||
|
}
|
||||||
|
if rawButton[0].line != 4 {
|
||||||
|
t.Fatalf("violation line = %d, want 4", rawButton[0].line)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCheckFrontendRawButtonExemptions(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
if err := os.MkdirAll(filepath.Join(dir, "components", "ui"), 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.MkdirAll(filepath.Join(dir, "lib"), 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The Button primitive itself renders a raw <button> — components/ui/ is exempt.
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, "components", "ui", "button.tsx"), []byte(`const Comp = asChild ? Slot : "button";
|
||||||
|
return <button className={cn(buttonVariants({ variant, size, className }))} ref={ref} {...props} />
|
||||||
|
`), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Non-React surfaces build DOM via HTML strings in plain .ts — <Button> does not exist there.
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, "lib", "iframe-script.ts"), []byte("const html = `<button class=\"bn-edit\">Edit</button>`\n"), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
violations, _ := checkFrontend(dir)
|
||||||
|
for _, v := range violations {
|
||||||
|
if v.rule == "no-raw-button" {
|
||||||
|
t.Fatalf("expected no no-raw-button violations, got %#v", v)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCheckPluginPagesFlagsRawButton(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, "settings.tsx"), []byte(`export function Settings() {
|
||||||
|
return <button onClick={connect}>Connect</button>
|
||||||
|
}
|
||||||
|
`), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
violations := checkPluginPages([]string{dir})
|
||||||
|
found := false
|
||||||
|
for _, v := range violations {
|
||||||
|
if v.rule == "no-raw-button-in-plugin" {
|
||||||
|
found = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !found {
|
||||||
|
t.Fatalf("expected no-raw-button-in-plugin violation, got %#v", violations)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestPluginRESTFileAllowed(t *testing.T) {
|
func TestPluginRESTFileAllowed(t *testing.T) {
|
||||||
if !pluginRESTFileAllowed("/tmp/repo/backend/internal/plugins/calcomblock/web/settings.tsx") {
|
if !pluginRESTFileAllowed("/tmp/repo/backend/internal/plugins/calcomblock/web/settings.tsx") {
|
||||||
t.Fatalf("expected documented calcom settings panel to be allowlisted")
|
t.Fatalf("expected documented calcom settings panel to be allowlisted")
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user