Skip to content

Commit fff3bcf

Browse files
authored
feat(style): add max-run-lines setting to detect long inline scripts (#11)
1 parent 323661c commit fff3bcf

6 files changed

Lines changed: 199 additions & 1 deletion

File tree

docs/configuration/linters.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ linters:
9999
naming-convention: "" # "title", "sentence", or ""
100100
checkout-first: false # Check if checkout is first step
101101
require-step-names: false # Require all steps to have names
102+
max-run-lines: 0 # Max lines in run scripts (0 = disabled)
102103
```
103104

104105
| Setting | Default | Description |
@@ -108,6 +109,7 @@ linters:
108109
| `naming-convention` | `""` | `"title"` (Every Word Uppercase), `"sentence"` (First word only), or `""` (none) |
109110
| `checkout-first` | `false` | Warn if checkout is not first step |
110111
| `require-step-names` | `false` | Require all steps to have names |
112+
| `max-run-lines` | `0` | Max lines in run scripts (0 = disabled) |
111113

112114
## Examples
113115

docs/linters/style.md

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Consistent styling:
2929
| **Name not first** | `name:` field not first in step definition |
3030
| **Checkout not first** | `actions/checkout` not the first step (optional) |
3131
| **Env shadowing** | Job-level env var shadows workflow-level env var |
32+
| **Run script too long** | Run script exceeds max lines (opt-in via `max-run-lines`) |
3233

3334
## Example Output
3435

@@ -38,6 +39,7 @@ ci.yml:5: (style) Job 'j1' has cryptic ID and is missing a name
3839
ci.yml:10: (style) Step is missing a name
3940
ci.yml:15: (style) Step 'name' should come first before other fields
4041
ci.yml:8: (style) Job env var 'NODE_ENV' shadows workflow-level env var
42+
ci.yml:20: (style) Run script has 15 lines (max 10); consider extracting to a script file
4143
```
4244

4345
## Auto-fix
@@ -57,6 +59,7 @@ linters:
5759
naming-convention: "" # "title", "sentence", or "" (default: none)
5860
checkout-first: false # Check checkout is first step (default: false)
5961
require-step-names: false # Require all steps to have names (default: false)
62+
max-run-lines: 0 # Max lines in run scripts (default: 0 = disabled)
6063
```
6164
6265
### min-name-length
@@ -120,6 +123,17 @@ When enabled, requires all steps to have explicit `name:` fields.
120123

121124
Simple steps like `- run: go build` often don't need names. Enable this for stricter naming policies.
122125

126+
### max-run-lines
127+
128+
Maximum allowed lines in a `run:` script before suggesting extraction to a script file.
129+
130+
| Value | Description |
131+
|-------|-------------|
132+
| `0` | Disabled (default) - no limit on run script length |
133+
| `10` | Warn if run script exceeds 10 lines |
134+
135+
Long inline scripts are harder to maintain and test. Consider extracting them to a script file (e.g., `scripts/build.sh`) called from the workflow.
136+
123137
## Cryptic Job ID Detection
124138

125139
A job ID is considered "cryptic" if it:
@@ -222,8 +236,38 @@ name: build and test
222236
name: Build And Test
223237
```
224238

239+
### Long Run Scripts
240+
241+
```yaml
242+
# With max-run-lines: 10
243+
linters:
244+
settings:
245+
style:
246+
max-run-lines: 10
247+
```
248+
249+
```yaml
250+
# Bad - inline script is too long
251+
- name: Build and Deploy
252+
run: |
253+
npm install
254+
npm run build
255+
npm run test
256+
docker build -t myapp .
257+
docker push myapp
258+
kubectl apply -f deploy.yaml
259+
kubectl rollout status deployment/myapp
260+
curl -X POST https://notify.example.com
261+
echo "Done deploying"
262+
./cleanup.sh
263+
# ... more lines
264+
265+
# Good - extract to script file
266+
- name: Build and Deploy
267+
run: ./scripts/build-and-deploy.sh
268+
```
269+
225270
## See Also
226271

227272
- [Linters Configuration](../configuration/linters) - Configure style settings
228273
- [format](format) - For formatting checks (indentation, line length)
229-

docs/usage/init.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ linters:
113113
naming-convention: ""
114114
checkout-first: false
115115
require-step-names: false
116+
max-run-lines: 0
116117
117118
upgrade:
118119
version: tag

internal/config/style_settings.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package config
33
const (
44
defaultMinNameLength = 3
55
defaultMaxNameLength = 50
6+
defaultMaxRunLines = 0 // 0 means disabled
67
)
78

89
// StyleSettings contains settings for the style linter.
@@ -20,13 +21,16 @@ type StyleSettings struct {
2021
CheckoutFirst bool `yaml:"checkout-first"`
2122
// RequireStepNames requires all steps to have explicit names
2223
RequireStepNames bool `yaml:"require-step-names"`
24+
// MaxRunLines is the maximum allowed lines in a run script (0 = disabled)
25+
MaxRunLines int `yaml:"max-run-lines"`
2326
}
2427

2528
// DefaultStyleSettings returns the default style linter settings.
2629
func DefaultStyleSettings() *StyleSettings {
2730
return &StyleSettings{
2831
MinNameLength: defaultMinNameLength,
2932
MaxNameLength: defaultMaxNameLength,
33+
MaxRunLines: defaultMaxRunLines,
3034
}
3135
}
3236

internal/linter/style_linter.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,11 @@ func (l *StyleLinter) checkSteps(wf *workflow.Workflow, job map[string]any, file
160160
if issue := l.checkNameFirst(lines, stepLine, file); issue != nil {
161161
issues = append(issues, issue)
162162
}
163+
164+
// Check run script length
165+
if issue := l.checkRunLength(step, file, stepLine); issue != nil {
166+
issues = append(issues, issue)
167+
}
163168
}
164169

165170
return issues
@@ -283,6 +288,27 @@ func (l *StyleLinter) checkNameFirst(lines []string, stepLine int, file string)
283288
return nil
284289
}
285290

291+
// checkRunLength checks if a run script exceeds the maximum line count.
292+
func (l *StyleLinter) checkRunLength(step map[string]any, file string, line int) *Issue {
293+
if l.settings.MaxRunLines <= 0 {
294+
return nil
295+
}
296+
297+
runScript, ok := step["run"].(string)
298+
if !ok || runScript == "" {
299+
return nil
300+
}
301+
302+
lineCount := strings.Count(strings.TrimSpace(runScript), "\n") + 1
303+
if lineCount > l.settings.MaxRunLines {
304+
msg := fmt.Sprintf("Run script has %d lines (max %d); consider extracting to a script file",
305+
lineCount, l.settings.MaxRunLines)
306+
return newIssue(file, line, msg)
307+
}
308+
309+
return nil
310+
}
311+
286312
// extractJobEnv extracts env variable names from a job map.
287313
func extractJobEnv(job map[string]any) map[string]bool {
288314
result := make(map[string]bool)

internal/linter/style_linter_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package linter
22

33
import (
4+
"fmt"
45
"os"
56
"path/filepath"
67
"strings"
@@ -842,3 +843,123 @@ jobs:
842843
t.Error("Expected to find 'Job name too short' issue")
843844
}
844845
}
846+
847+
func TestStyleLinter_RunScriptLength(t *testing.T) {
848+
tests := []struct {
849+
name string
850+
maxLines int
851+
runScript string
852+
wantFlagged bool
853+
}{
854+
{
855+
name: "too long",
856+
maxLines: 5,
857+
runScript: "echo 1\necho 2\necho 3\necho 4\necho 5\necho 6",
858+
wantFlagged: true,
859+
},
860+
{
861+
name: "within limit",
862+
maxLines: 5,
863+
runScript: "echo 1\necho 2\necho 3",
864+
wantFlagged: false,
865+
},
866+
{
867+
name: "check disabled",
868+
maxLines: 0,
869+
runScript: "echo 1\necho 2\necho 3\necho 4\necho 5\necho 6\necho 7\necho 8\necho 9\necho 10",
870+
wantFlagged: false,
871+
},
872+
{
873+
name: "single line",
874+
maxLines: 1,
875+
runScript: "echo hello",
876+
wantFlagged: false,
877+
},
878+
{
879+
name: "exact boundary",
880+
maxLines: 5,
881+
runScript: "echo 1\necho 2\necho 3\necho 4\necho 5",
882+
wantFlagged: false,
883+
},
884+
}
885+
886+
for _, tt := range tests {
887+
t.Run(tt.name, func(t *testing.T) {
888+
tmpDir := t.TempDir()
889+
workflowPath := filepath.Join(tmpDir, "test.yml")
890+
891+
content := fmt.Sprintf(`name: Test Workflow
892+
on: push
893+
jobs:
894+
build:
895+
runs-on: ubuntu-latest
896+
steps:
897+
- name: Test Step
898+
run: |
899+
%s
900+
`, strings.ReplaceAll(tt.runScript, "\n", "\n "))
901+
902+
if err := os.WriteFile(workflowPath, []byte(content), 0600); err != nil {
903+
t.Fatalf("Failed to write test workflow: %v", err)
904+
}
905+
906+
wf, err := workflow.LoadWorkflow(workflowPath)
907+
if err != nil {
908+
t.Fatalf("LoadWorkflow() error = %v", err)
909+
}
910+
911+
linter := NewStyleLinter(&config.StyleSettings{MaxRunLines: tt.maxLines})
912+
issues, err := linter.LintWorkflow(wf)
913+
if err != nil {
914+
t.Fatalf("LintWorkflow() error = %v", err)
915+
}
916+
917+
found := false
918+
for _, issue := range issues {
919+
if strings.Contains(issue.Message, "Run script has") {
920+
found = true
921+
break
922+
}
923+
}
924+
925+
if found != tt.wantFlagged {
926+
t.Errorf("got flagged=%v, want flagged=%v", found, tt.wantFlagged)
927+
}
928+
})
929+
}
930+
}
931+
932+
func TestStyleLinter_RunScriptStepWithUsesOnly(t *testing.T) {
933+
tmpDir := t.TempDir()
934+
workflowPath := filepath.Join(tmpDir, "test.yml")
935+
936+
content := `name: Test Workflow
937+
on: push
938+
jobs:
939+
build:
940+
runs-on: ubuntu-latest
941+
steps:
942+
- name: Checkout
943+
uses: actions/checkout@v4
944+
`
945+
if err := os.WriteFile(workflowPath, []byte(content), 0600); err != nil {
946+
t.Fatalf("Failed to write test workflow: %v", err)
947+
}
948+
949+
wf, err := workflow.LoadWorkflow(workflowPath)
950+
if err != nil {
951+
t.Fatalf("LoadWorkflow() error = %v", err)
952+
}
953+
954+
linter := NewStyleLinter(&config.StyleSettings{MaxRunLines: 1})
955+
issues, err := linter.LintWorkflow(wf)
956+
if err != nil {
957+
t.Fatalf("LintWorkflow() error = %v", err)
958+
}
959+
960+
for _, issue := range issues {
961+
if strings.Contains(issue.Message, "Run script has") {
962+
t.Error("Did not expect 'uses' step to trigger run script check")
963+
}
964+
}
965+
}

0 commit comments

Comments
 (0)