Skip to content

Commit ab527ae

Browse files
markokajzermarkokajzer
committed
refactor: integrate erb-no-javascript-tag-helper 
1 parent 14743fe commit ab527ae

File tree

11 files changed

+138
-170
lines changed

11 files changed

+138
-170
lines changed

‎docs/docs/blog/whats-new-in-herb-v0-9.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ This release also introduces two new rule categories: **`erb-safety-*`** rules e
414414
##### ERB Safety Rules
415415

416416
- [`erb-no-statement-in-script`](/linter/rules/erb-no-statement-in-script.md)<br/>Avoid `<% %>` tags inside `<script>`. Use `<%= %>` to interpolate values.
417-
- [`erb-no-javascript-tag-helper`](/linter/rules/erb-no-javascript-tag-helper.md)<br/>Avoid `javascript_tag`. Use inline `<script>` tags instead.
417+
- [`erb-no-javascript-tag-helper`](/linter/rules/erb-disallow-inline-scripts.md)<br/>Avoid `javascript_tag`. Use inline `<script>` tags instead.
418418
- [`erb-no-unsafe-script-interpolation`](/linter/rules/erb-no-unsafe-script-interpolation.md)<br/>ERB output in `<script>` tags must use `.to_json` to safely serialize values.
419419
- [`erb-no-raw-output-in-attribute-value`](/linter/rules/erb-no-raw-output-in-attribute-value.md)<br/>Avoid `<%==` in attribute values. Use `<%= %>` instead.
420420
- [`erb-no-unsafe-raw`](/linter/rules/erb-no-unsafe-raw.md)<br/>Avoid `raw()` and `.html_safe` in ERB output.

‎javascript/packages/linter/docs/rules/README.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ This page contains documentation for all Herb Linter rules.
2323
#### ERB
2424

2525
- [`erb-comment-syntax`](./erb-comment-syntax.md) - Disallow Ruby comments immediately after ERB tags
26+
- [`erb-disallow-inline-scripts`](./erb-disallow-inline-scripts.md) - Disallow inline `<script>` tags, `javascript_tag`, and event handler attributes
2627
- [`erb-no-case-node-children`](./erb-no-case-node-children.md) - Don't use `children` for `case/when` and `case/in` nodes
2728
- [`erb-no-debug-output`](./erb-no-debug-output.md) - Disallow debug output methods (`p`, `pp`, `puts`, `print`, `debug`) in ERB templates
2829
- [`erb-no-conditional-html-element`](./erb-no-conditional-html-element.md) - Disallow conditional HTML elements
@@ -34,7 +35,6 @@ This page contains documentation for all Herb Linter rules.
3435
- [`erb-no-inline-case-conditions`](./erb-no-inline-case-conditions.md) - Disallow inline `case`/`when` and `case`/`in` conditions in a single ERB tag
3536
- [`erb-no-instance-variables-in-partials`](./erb-no-instance-variables-in-partials.md) - Disallow instance variables in partials
3637
- [`erb-no-interpolated-class-names`](./erb-no-interpolated-class-names.md) - Disallow ERB interpolation inside CSS class names
37-
- [`erb-no-javascript-tag-helper`](./erb-no-javascript-tag-helper.md) - Disallow `javascript_tag` helper
3838
- [`erb-no-output-control-flow`](./erb-no-output-control-flow.md) - Prevents outputting control flow blocks
3939
- [`erb-no-output-in-attribute-name`](./erb-no-output-in-attribute-name.md) - Disallow ERB output in attribute names
4040
- [`erb-no-output-in-attribute-position`](./erb-no-output-in-attribute-position.md) - Disallow ERB output in attribute position
@@ -83,7 +83,6 @@ This page contains documentation for all Herb Linter rules.
8383
- [`html-body-only-elements`](./html-body-only-elements.md) - Require content elements inside `<body>`.
8484
- [`html-boolean-attributes-no-value`](./html-boolean-attributes-no-value.md) - Prevents values on boolean attributes
8585
- [`html-details-has-summary`](./html-details-has-summary.md) - Require `<summary>` in `<details>` elements
86-
- [`html-disallow-inline-scripts`](./html-disallow-inline-scripts.md) - Disallow inline `<script>` tags and event handler attributes
8786
- [`html-head-only-elements`](./html-head-only-elements.md) - Require head-scoped elements inside `<head>`.
8887
- [`html-iframe-has-title`](./html-iframe-has-title.md) - `iframe` elements must have a `title` attribute
8988
- [`html-img-require-alt`](./html-img-require-alt.md) - Requires `alt` attributes on `<img>` tags

javascript/packages/linter/docs/rules/html-disallow-inline-scripts.md renamed to javascript/packages/linter/docs/rules/erb-disallow-inline-scripts.md

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Linter Rule: Disallow inline script tags and event handler attributes
22

3-
**Rule:** `html-disallow-inline-scripts`
3+
**Rule:** `erb-disallow-inline-scripts`
44

55
## Description
66

@@ -15,6 +15,8 @@ All JavaScript should be included via external assets to support strong CSP poli
1515
This rule enforces:
1616

1717
- No `<script>` tags embedded directly in templates.
18+
- No `javascript_tag` Rails helper usage.
19+
- No `content_tag :script` or `tag.script` Rails helper usage.
1820
- No event handler attributes (`onclick`, `onmouseover`, etc.).
1921

2022
## Examples
@@ -61,6 +63,24 @@ This rule enforces:
6163
</script>
6264
```
6365

66+
```erb
67+
<%= javascript_tag do %>
68+
if (a < 1) { <%= unsafe %> }
69+
<% end %>
70+
```
71+
72+
```erb
73+
<%= content_tag :script do %>
74+
alert('Hello')
75+
<% end %>
76+
```
77+
78+
```erb
79+
<%= tag.script do %>
80+
alert('Hello')
81+
<% end %>
82+
```
83+
6484
```erb
6585
<button onclick="doSomething()">Click</button>
6686
```
@@ -79,6 +99,7 @@ This rule enforces:
7999

80100
## References
81101

102+
- [Shopify/better-html — `NoJavascriptTagHelper`](https://github.com/Shopify/better-html/blob/main/lib/better_html/test_helper/safe_erb/no_javascript_tag_helper.rb)
82103
- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o)
83104
- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP)
84105
- [wooorm/html-event-attributes](https://github.com/wooorm/html-event-attributes)

‎javascript/packages/linter/docs/rules/erb-no-javascript-tag-helper.md‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎javascript/packages/linter/src/rules.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { ActionViewStrictLocalsFirstLineRule } from "./rules/actionview-strict-l
1212
import { ActionViewStrictLocalsPartialOnlyRule } from "./rules/actionview-strict-locals-partial-only.js"
1313

1414
import { ERBCommentSyntax } from "./rules/erb-comment-syntax.js";
15+
import { ERBDisallowInlineScriptsRule } from "./rules/erb-disallow-inline-scripts.js"
1516
import { ERBNoCaseNodeChildrenRule } from "./rules/erb-no-case-node-children.js"
1617
import { ERBNoDebugOutputRule } from "./rules/erb-no-debug-output.js"
1718
import { ERBNoConditionalHTMLElementRule } from "./rules/erb-no-conditional-html-element.js"
@@ -24,7 +25,6 @@ import { ERBNoExtraWhitespaceRule } from "./rules/erb-no-extra-whitespace-inside
2425
import { ERBNoInlineCaseConditionsRule } from "./rules/erb-no-inline-case-conditions.js"
2526
import { ERBNoInstanceVariablesInPartialsRule } from "./rules/erb-no-instance-variables-in-partials.js"
2627
import { ERBNoInterpolatedClassNamesRule } from "./rules/erb-no-interpolated-class-names.js"
27-
import { ERBNoJavascriptTagHelperRule } from "./rules/erb-no-javascript-tag-helper.js"
2828
import { ERBNoOutputControlFlowRule } from "./rules/erb-no-output-control-flow.js"
2929
import { ERBNoOutputInAttributeNameRule } from "./rules/erb-no-output-in-attribute-name.js"
3030
import { ERBNoOutputInAttributePositionRule } from "./rules/erb-no-output-in-attribute-position.js"
@@ -67,7 +67,6 @@ import { HTMLAvoidBothDisabledAndAriaDisabledRule } from "./rules/html-avoid-bot
6767
import { HTMLBodyOnlyElementsRule } from "./rules/html-body-only-elements.js"
6868
import { HTMLBooleanAttributesNoValueRule } from "./rules/html-boolean-attributes-no-value.js"
6969
import { HTMLDetailsHasSummaryRule } from "./rules/html-details-has-summary.js"
70-
import { HTMLDisallowInlineScriptsRule } from "./rules/html-disallow-inline-scripts.js"
7170
import { HTMLHeadOnlyElementsRule } from "./rules/html-head-only-elements.js"
7271
import { HTMLIframeHasTitleRule } from "./rules/html-iframe-has-title.js"
7372
import { HTMLImgRequireAltRule } from "./rules/html-img-require-alt.js"
@@ -115,6 +114,7 @@ export const rules: RuleClass[] = [
115114
ActionViewStrictLocalsPartialOnlyRule,
116115

117116
ERBCommentSyntax,
117+
ERBDisallowInlineScriptsRule,
118118
ERBNoCaseNodeChildrenRule,
119119
ERBNoDebugOutputRule,
120120
ERBNoEmptyControlFlowRule,
@@ -127,7 +127,6 @@ export const rules: RuleClass[] = [
127127
ERBNoInlineCaseConditionsRule,
128128
ERBNoInstanceVariablesInPartialsRule,
129129
ERBNoInterpolatedClassNamesRule,
130-
ERBNoJavascriptTagHelperRule,
131130
ERBNoOutputControlFlowRule,
132131
ERBNoOutputInAttributeNameRule,
133132
ERBNoOutputInAttributePositionRule,
@@ -170,7 +169,6 @@ export const rules: RuleClass[] = [
170169
HTMLBodyOnlyElementsRule,
171170
HTMLBooleanAttributesNoValueRule,
172171
HTMLDetailsHasSummaryRule,
173-
HTMLDisallowInlineScriptsRule,
174172
HTMLHeadOnlyElementsRule,
175173
HTMLIframeHasTitleRule,
176174
HTMLImgRequireAltRule,

javascript/packages/linter/src/rules/html-disallow-inline-scripts.ts renamed to javascript/packages/linter/src/rules/erb-disallow-inline-scripts.ts

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1-
import { ParserRule } from "../types.js"
2-
import { BaseRuleVisitor, isJavaScriptTagElement } from "./rule-utils.js"
3-
import { getTagLocalName, getAttributeName } from "@herb-tools/core"
1+
import { getTagLocalName, getAttributeName, isERBOpenTagNode } from "@herb-tools/core"
2+
import type { ParseResult, ParserOptions, HTMLElementNode, HTMLAttributeNode } from "@herb-tools/core"
43

4+
import { BaseRuleVisitor, isJavaScriptTagElement } from "./rule-utils.js"
55
import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
6-
import type { ParseResult, HTMLElementNode, HTMLAttributeNode } from "@herb-tools/core"
6+
import { ParserRule } from "../types.js"
7+
8+
const JAVASCRIPT_TAG_ELEMENT_SOURCE = "ActionView::Helpers::JavaScriptHelper#javascript_tag"
9+
const JAVASCRIPT_INCLUDE_TAG_ELEMENT_SOURCE = "ActionView::Helpers::AssetTagHelper#javascript_include_tag"
710

811
const HTML_EVENT_ATTRIBUTES = new Set([
912
// Window Event Attributes
@@ -112,7 +115,7 @@ const HTML_EVENT_ATTRIBUTES = new Set([
112115
"ontoggle",
113116
])
114117

115-
class HTMLDisallowInlineScriptsVisitor extends BaseRuleVisitor {
118+
class ERBDisallowInlineScriptsVisitor extends BaseRuleVisitor {
116119
visitHTMLElementNode(node: HTMLElementNode): void {
117120
if (getTagLocalName(node) === "script") {
118121
this.checkInlineScript(node)
@@ -137,15 +140,34 @@ class HTMLDisallowInlineScriptsVisitor extends BaseRuleVisitor {
137140
private checkInlineScript(node: HTMLElementNode): void {
138141
if (!isJavaScriptTagElement(node)) return
139142

140-
this.addOffense(
141-
"Avoid inline `<script>` tags. Use `javascript_include_tag` to include external JavaScript files instead.",
142-
node.open_tag!.location,
143-
)
143+
if (this.isJavascriptTagElement(node)) {
144+
this.addOffense(
145+
"Avoid `javascript_tag`. Use `javascript_include_tag` to include external JavaScript files instead.",
146+
node.open_tag!.location,
147+
)
148+
} else if (!this.isJavascriptIncludeTagElement(node)) {
149+
this.addOffense(
150+
"Avoid inline `<script>` tags. Use `javascript_include_tag` to include external JavaScript files instead.",
151+
node.open_tag!.location,
152+
)
153+
}
154+
}
155+
156+
private isJavascriptTagElement(node: HTMLElementNode): boolean {
157+
if (!isERBOpenTagNode(node.open_tag)) return false
158+
159+
return node.element_source === JAVASCRIPT_TAG_ELEMENT_SOURCE
160+
}
161+
162+
private isJavascriptIncludeTagElement(node: HTMLElementNode): boolean {
163+
if (!isERBOpenTagNode(node.open_tag)) return false
164+
165+
return node.element_source === JAVASCRIPT_INCLUDE_TAG_ELEMENT_SOURCE
144166
}
145167
}
146168

147-
export class HTMLDisallowInlineScriptsRule extends ParserRule {
148-
static ruleName = "html-disallow-inline-scripts"
169+
export class ERBDisallowInlineScriptsRule extends ParserRule {
170+
static ruleName = "erb-disallow-inline-scripts"
149171
static introducedIn = this.version("unreleased")
150172

151173
get defaultConfig(): FullRuleConfig {
@@ -155,8 +177,14 @@ export class HTMLDisallowInlineScriptsRule extends ParserRule {
155177
}
156178
}
157179

180+
get parserOptions(): Partial<ParserOptions> {
181+
return {
182+
action_view_helpers: true,
183+
}
184+
}
185+
158186
check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
159-
const visitor = new HTMLDisallowInlineScriptsVisitor(this.ruleName, context)
187+
const visitor = new ERBDisallowInlineScriptsVisitor(this.ruleName, context)
160188

161189
visitor.visit(result.value)
162190

‎javascript/packages/linter/src/rules/erb-no-javascript-tag-helper.ts‎

Lines changed: 0 additions & 54 deletions
This file was deleted.

‎javascript/packages/linter/src/rules/index.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export * from "./actionview-strict-locals-first-line.js"
1717
export * from "./actionview-strict-locals-partial-only.js"
1818

1919
export * from "./erb-comment-syntax.js"
20+
export * from "./erb-disallow-inline-scripts.js"
2021
export * from "./erb-no-case-node-children.js"
2122
export * from "./erb-no-debug-output.js"
2223
export * from "./erb-no-conditional-open-tag.js"
@@ -27,7 +28,6 @@ export * from "./erb-no-extra-newline.js"
2728
export * from "./erb-no-extra-whitespace-inside-tags.js"
2829
export * from "./erb-no-inline-case-conditions.js"
2930
export * from "./erb-no-instance-variables-in-partials.js"
30-
export * from "./erb-no-javascript-tag-helper.js"
3131
export * from "./erb-no-output-control-flow.js"
3232
export * from "./erb-no-output-in-attribute-name.js"
3333
export * from "./erb-no-output-in-attribute-position.js"
@@ -69,7 +69,6 @@ export * from "./html-avoid-both-disabled-and-aria-disabled.js"
6969
export * from "./html-body-only-elements.js"
7070
export * from "./html-boolean-attributes-no-value.js"
7171
export * from "./html-details-has-summary.js"
72-
export * from "./html-disallow-inline-scripts.js"
7372
export * from "./html-head-only-elements.js"
7473
export * from "./html-iframe-has-title.js"
7574
export * from "./html-img-require-alt.js"

0 commit comments

Comments
 (0)