Merge PR #760: fix(cli): skip non-Markdown files named explicitly on check/fix

Author: merge-queue-bot

cmd/mdsmith/check.go

@@ -113,6 +113,7 @@ func parseCheckFlags(args []string) (checkCLIOpts, []string, bool, int) {
 func checkFiles(fileArgs []string, opts checkCLIOpts) int {
 	cfg, cfgPath, logger, files, maxBytes, code := loadAndResolve(
 		fileArgs, opts.configPath, opts.verbose, opts.walk, opts.maxInputSize,
+		nonMarkdownSkipWarner(os.Stderr, opts.format, opts.quiet),
 	)
 	if code >= 0 {
 		return code

cmd/mdsmith/e2e_nonmarkdown_test.go

@@ -0,0 +1,115 @@
+package main_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// gitattributesFixture is a .gitattributes body whose `#` comment lines
+// would be parsed as ATX headings if linted as Markdown: they sit with
+// no surrounding blank lines (blank-line-around-headings would insert
+// them) and end in punctuation (MDS017). If the explicit-path guard
+// regresses, `fix` rewrites this content and the byte-equality
+// assertion fails.
+const gitattributesFixture = `# BEGIN mdsmith merge-driver
+*.md merge=mdsmith
+*.markdown merge=mdsmith
+# END mdsmith merge-driver
+# Re-enable git 3-way merge for code under the trees mdsmith marks -merge.
+# mdsmith derives those -merge lines from the config ignore patterns.
+`
+
+// TestFix_ExplicitNonMarkdownFileUnchanged is the regression test for
+// issue #759: passing a non-Markdown file explicitly to `mdsmith fix`
+// must not rewrite it. The directory walk already skips such files; an
+// explicit path must behave the same.
+func TestFix_ExplicitNonMarkdownFileUnchanged(t *testing.T) {
+	dir := t.TempDir()
+	isolateDir(t, dir)
+	attrPath := filepath.Join(dir, ".gitattributes")
+	require.NoError(t, os.WriteFile(attrPath, []byte(gitattributesFixture), 0o644))
+
+	_, stderr, exitCode := runBinaryInDir(t, dir, "", "fix", ".gitattributes")
+	assert.Equal(t, 0, exitCode, "fix on a non-Markdown file should succeed with nothing to do")
+	assert.Contains(t, stderr, `skipping ".gitattributes"`,
+		"fix should warn that the explicitly named non-Markdown file was skipped, not do nothing silently")
+
+	got, err := os.ReadFile(attrPath)
+	require.NoError(t, err)
+	assert.Equal(t, gitattributesFixture, string(got),
+		"fix must leave a non-Markdown file byte-for-byte unchanged")
+}
+
+// TestCheck_ExplicitNonMarkdownFileClean is the check-side counterpart:
+// a non-Markdown file named explicitly is skipped (no Markdown content
+// diagnostics, clean exit) but reported with a warning rather than a
+// silent no-op.
+func TestCheck_ExplicitNonMarkdownFileClean(t *testing.T) {
+	dir := t.TempDir()
+	isolateDir(t, dir)
+	attrPath := filepath.Join(dir, ".gitattributes")
+	require.NoError(t, os.WriteFile(attrPath, []byte(gitattributesFixture), 0o644))
+
+	stdout, stderr, exitCode := runBinaryInDir(t, dir, "", "check", ".gitattributes")
+	assert.Equal(t, 0, exitCode,
+		"check on a non-Markdown file should exit clean; stdout=%q stderr=%q", stdout, stderr)
+	assert.NotContains(t, stderr, "MDS", "no Markdown diagnostics should be reported")
+	assert.Contains(t, stderr, `skipping ".gitattributes"`,
+		"check should warn that the explicitly named non-Markdown file was skipped")
+}
+
+// TestCheck_NonMarkdownWarningSuppressed verifies the skip warning is a
+// text-format, non-quiet affordance only: it must not appear under
+// --quiet (non-error output) or --format json (the JSON document shares
+// the stderr stream and a prose line would corrupt it).
+func TestCheck_NonMarkdownWarningSuppressed(t *testing.T) {
+	dir := t.TempDir()
+	isolateDir(t, dir)
+	attrPath := filepath.Join(dir, ".gitattributes")
+	require.NoError(t, os.WriteFile(attrPath, []byte(gitattributesFixture), 0o644))
+
+	for _, tc := range []struct {
+		name string
+		args []string
+	}{
+		{"quiet", []string{"check", "--quiet", ".gitattributes"}},
+		{"json", []string{"check", "-f", "json", ".gitattributes"}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			stdout, stderr, exitCode := runBinaryInDir(t, dir, "", tc.args...)
+			assert.Equal(t, 0, exitCode, "should still exit clean; stdout=%q stderr=%q", stdout, stderr)
+			assert.NotContains(t, stderr, "skipping",
+				"the human skip warning must be suppressed in %s mode", tc.name)
+		})
+	}
+}
+
+// TestCheck_MixedExplicitPathsLintsOnlyMarkdown pins that when both a
+// Markdown file and a non-Markdown file are named explicitly, only the
+// Markdown one is linted: the non-Markdown file is dropped, but the
+// Markdown file's genuine diagnostic still surfaces.
+func TestCheck_MixedExplicitPathsLintsOnlyMarkdown(t *testing.T) {
+	dir := t.TempDir()
+	isolateDir(t, dir)
+	attrPath := filepath.Join(dir, ".gitattributes")
+	mdPath := filepath.Join(dir, "bad.md")
+	require.NoError(t, os.WriteFile(attrPath, []byte(gitattributesFixture), 0o644))
+	// A heading ending in a period is MDS017, a default-enabled rule
+	// (the issue confirms it fires); used only to prove the Markdown
+	// file was actually linted.
+	require.NoError(t, os.WriteFile(mdPath, []byte("# Heading.\n"), 0o644))
+
+	stdout, stderr, exitCode := runBinaryInDir(t, dir, "", "check", ".gitattributes", "bad.md")
+	assert.Equal(t, 1, exitCode,
+		"the Markdown file's diagnostic should still fail the run; stdout=%q stderr=%q", stdout, stderr)
+	assert.Contains(t, stderr, "bad.md", "the Markdown file should be the one reported")
+	// The non-Markdown file may appear in the skip warning
+	// (`skipping ".gitattributes"`), but must never appear as a linted
+	// diagnostic — those are anchored as `<path>:<line>:<col>`.
+	assert.NotContains(t, stderr, ".gitattributes:",
+		"the non-Markdown file must not be linted (no diagnostic anchored to it)")
+}

cmd/mdsmith/fix.go

@@ -226,6 +226,7 @@ type fixCLIOpts struct {
 func fixFiles(fileArgs []string, opts fixCLIOpts) int {
 	cfg, cfgPath, logger, files, maxBytes, code := loadAndResolve(
 		fileArgs, opts.configPath, opts.verbose, opts.walk, opts.maxInputSize,
+		nonMarkdownSkipWarner(os.Stderr, opts.format, opts.quiet),
 	)
 	if code >= 0 {
 		return code

cmd/mdsmith/main.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime/debug"
+	"strings"
 
 	flag "github.com/spf13/pflag"
 
@@ -15,6 +16,7 @@ import (
 	"github.com/jeduden/mdsmith/internal/discovery"
 	"github.com/jeduden/mdsmith/internal/lint"
 	vlog "github.com/jeduden/mdsmith/internal/log"
+	"github.com/jeduden/mdsmith/internal/mdpath"
 	"github.com/jeduden/mdsmith/internal/output"
 	"github.com/jeduden/mdsmith/internal/profiling"
 	"github.com/jeduden/mdsmith/internal/query"
@@ -385,7 +387,7 @@ func printRunStatsTo(w io.Writer, format string, quiet bool, stats runStats) {
 func loadAndResolve(
 	fileArgs []string, configPath string,
 	verbose bool, walk walkCLI,
-	maxInputSize string,
+	maxInputSize string, onSkipNonMarkdown func(string),
 ) (*config.Config, string, *vlog.Logger, []string, int64, int) {
 	logger := &vlog.Logger{Enabled: verbose, W: os.Stderr}
 
@@ -399,6 +401,7 @@ func loadAndResolve(
 	}
 
 	opts := resolveOpts(cfg, walk)
+	opts.OnSkipNonMarkdown = onSkipNonMarkdown
 	files, err := lint.ResolveFilesWithOpts(fileArgs, opts)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "mdsmith: %v\n", err)
@@ -417,6 +420,34 @@ func loadAndResolve(
 	return cfg, cfgPath, logger, files, maxBytes, -1
 }
 
+// nonMarkdownSkipWarner returns the OnSkipNonMarkdown hook for check and
+// fix: a function that writes one stderr line per explicitly named
+// non-Markdown file that was skipped, so `mdsmith fix .gitattributes` is
+// a visible no-op instead of a silent one (issue #759).
+//
+// It returns nil — disabling the notification entirely — when the run is
+// --quiet (a skipped file is non-error output) or the format is not
+// text. check and fix emit their diagnostics, including `--format json`
+// and `--format sarif`, on stderr; a prose warning on the same stream
+// would corrupt that structured output, so the human notice is limited
+// to the text format. Repeated names are de-duplicated so a doubled
+// argument does not double the warning.
+func nonMarkdownSkipWarner(w io.Writer, format string, quiet bool) func(string) {
+	if quiet || format != "text" {
+		return nil
+	}
+	exts := strings.Join(mdpath.Extensions(), ", ")
+	seen := make(map[string]struct{})
+	return func(path string) {
+		if _, dup := seen[path]; dup {
+			return
+		}
+		seen[path] = struct{}{}
+		// Write error swallowed: see printErrorsTo rationale.
+		_, _ = fmt.Fprintf(w, "mdsmith: skipping %q: not a Markdown file (expected %s)\n", path, exts)
+	}
+}
+
 // splitStdinArg separates a "-" argument (stdin) from file arguments.
 // Returns true if "-" was found and the remaining file arguments.
 func splitStdinArg(args []string) (hasStdin bool, fileArgs []string) {

cmd/mdsmith/main_unit_test.go

@@ -1630,3 +1630,38 @@ func TestReportFixResultTo_LargeDiagWriteErrorReturns2(t *testing.T) {
 	code := reportFixResultTo(opts, result, &vlog.Logger{}, &alwaysErrorWriter{})
 	assert.Equal(t, 2, code)
 }
+
+func TestNonMarkdownSkipWarner_TextEmitsAndDedupes(t *testing.T) {
+	var buf bytes.Buffer
+	warn := nonMarkdownSkipWarner(&buf, "text", false)
+	require.NotNil(t, warn, "text, non-quiet should produce an active warner")
+
+	warn(".gitattributes")
+	warn(".gitattributes") // duplicate: must not warn twice
+	warn("Makefile")
+
+	out := buf.String()
+	assert.Equal(t, 1, strings.Count(out, `skipping ".gitattributes"`),
+		"a repeated path must warn only once")
+	assert.Contains(t, out, `skipping "Makefile"`)
+	// The recognized extensions are surfaced so a user with a
+	// near-miss name (e.g. notes.mdown) understands why it was skipped.
+	assert.Contains(t, out, ".md, .markdown")
+}
+
+func TestNonMarkdownSkipWarner_SuppressedForQuietAndNonText(t *testing.T) {
+	for _, tc := range []struct {
+		name   string
+		format string
+		quiet  bool
+	}{
+		{"quiet", "text", true},
+		{"json", "json", false},
+		{"sarif", "sarif", false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Nil(t, nonMarkdownSkipWarner(io.Discard, tc.format, tc.quiet),
+				"warner must be disabled to avoid non-error output / corrupting structured formats")
+		})
+	}
+}

docs/reference/cli/check.md

@@ -19,6 +19,13 @@ read from stdin. With no file arguments, files are
 discovered from `.mdsmith.yml` `files:` patterns
 (default: `**/*.md`, `**/*.markdown`).
 
+Only Markdown files are linted. A non-Markdown path
+(such as `.gitattributes`) is skipped whether the walk
+reaches it or you name it explicitly. Naming one
+explicitly prints a `skipping …: not a Markdown file`
+warning on stderr, so the skip is never a silent no-op;
+`--quiet` and the `json`/`sarif` formats suppress it.
+
 ## Flags
 
 | Flag                | Default | Description                            |

docs/reference/cli/fix.md

@@ -16,6 +16,13 @@ Files can be paths, directories (walked recursively for
 rejected — files must be writable. With no file arguments,
 files are discovered from `.mdsmith.yml` `files:` patterns.
 
+Only Markdown files are fixed. A non-Markdown path (such
+as `.gitattributes`) is skipped whether the walk reaches
+it or you name it explicitly, so `fix` never rewrites it.
+Naming one explicitly prints a `skipping …: not a
+Markdown file` warning on stderr; `--quiet` and the
+`json`/`sarif` formats suppress it.
+
 ## Flags
 
 | Flag                | Default | Description                            |

internal/lint/files.go

@@ -89,6 +89,16 @@ type ResolveOpts struct {
 	// root) as well as FIFOs, devices, and sockets (reading them
 	// during linting could block or fail unexpectedly).
 	FollowSymlinks bool
+
+	// OnSkipNonMarkdown, when non-nil, is called with each
+	// explicitly named path dropped because it is not Markdown, so a
+	// caller can warn instead of leaving `mdsmith fix .gitattributes`
+	// a silent no-op (issue #759). It fires ONLY for a directly named
+	// file: entries filtered out during a directory walk or glob
+	// expansion are skipped silently, because the user named the
+	// directory or pattern, not the individual file. The argument is
+	// the path exactly as it was passed.
+	OnSkipNonMarkdown func(path string)
 }
 
 // DefaultResolveOpts returns options with defaults applied.
@@ -211,7 +221,24 @@ func resolveArg(arg string, opts ResolveOpts, addFile func(string)) error {
 		return nil
 	}
 
-	// Explicitly named files are never filtered by gitignore.
+	// Skip files that are not Markdown even when named explicitly, so
+	// an explicit path matches the walk (walkDir) and glob (resolveGlob)
+	// branches, both of which gate on isMarkdown. Without this, `mdsmith
+	// check .gitattributes` lints a git-config file as Markdown and
+	// `mdsmith fix` rewrites it — including the merge-driver block
+	// mdsmith generates there (issue #759). The extension is the only
+	// signal available at resolve time; content sniffing would be both
+	// slower and ambiguous. Unlike the walk and glob paths, a directly
+	// named file gets an OnSkipNonMarkdown notification so the caller
+	// can warn rather than silently doing nothing.
+	if !isMarkdown(arg) {
+		if opts.OnSkipNonMarkdown != nil {
+			opts.OnSkipNonMarkdown(arg)
+		}
+		return nil
+	}
+
+	// Explicitly named Markdown files are never filtered by gitignore.
 	addFile(arg)
 	return nil
 }

internal/lint/files_test.go

@@ -29,10 +29,61 @@ func TestResolveFiles_NonMarkdownFile(t *testing.T) {
 	txtFile := filepath.Join(dir, "test.txt")
 	require.NoError(t, os.WriteFile(txtFile, []byte("hello"), 0o644))
 
-	// Non-markdown files are still returned when given explicitly as args.
+	// A non-Markdown file named explicitly is skipped, matching the
+	// directory walk and glob expansion — mdsmith cannot meaningfully
+	// lint a file that is not Markdown, and fixing one rewrites content
+	// it never should touch (issue #759, e.g. .gitattributes).
 	files, err := ResolveFiles([]string{txtFile})
 	require.NoError(t, err)
+	require.Empty(t, files)
+}
+
+// TestResolveFiles_MixedMarkdownAndNonMarkdown pins that an explicit
+// argument list keeps its Markdown members and drops the rest, so a
+// caller that names both a real doc and a config file (or shell-globs a
+// wider set) still lints exactly the Markdown ones. Regression guard for
+// issue #759.
+func TestResolveFiles_MixedMarkdownAndNonMarkdown(t *testing.T) {
+	dir := t.TempDir()
+	mdFile := filepath.Join(dir, "doc.md")
+	txtFile := filepath.Join(dir, "notes.txt")
+	attrFile := filepath.Join(dir, ".gitattributes")
+	require.NoError(t, os.WriteFile(mdFile, []byte("# Hello"), 0o644))
+	require.NoError(t, os.WriteFile(txtFile, []byte("hello"), 0o644))
+	require.NoError(t, os.WriteFile(attrFile, []byte("*.md merge=mdsmith\n"), 0o644))
+
+	files, err := ResolveFiles([]string{mdFile, txtFile, attrFile})
+	require.NoError(t, err)
 	require.Len(t, files, 1)
+	assert.Equal(t, mdFile, files[0])
+}
+
+// TestResolveFiles_OnSkipNonMarkdown_ExplicitOnly verifies the
+// OnSkipNonMarkdown hook fires once per explicitly named non-Markdown
+// file (so the CLI can warn instead of a silent no-op), but never for a
+// Markdown file, and never for non-Markdown entries filtered out by a
+// directory walk — the walk skips those by design and the user did not
+// name them. Guards the warning wiring for issue #759.
+func TestResolveFiles_OnSkipNonMarkdown_ExplicitOnly(t *testing.T) {
+	dir := t.TempDir()
+	mdFile := filepath.Join(dir, "doc.md")
+	txtFile := filepath.Join(dir, "notes.txt")
+	require.NoError(t, os.WriteFile(mdFile, []byte("# Hello"), 0o644))
+	require.NoError(t, os.WriteFile(txtFile, []byte("hello"), 0o644))
+	// A non-Markdown file inside a walked directory must NOT trigger
+	// the hook: the walk drops it silently and the user named the dir,
+	// not the file.
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "walked.txt"), []byte("x"), 0o644))
+
+	var skipped []string
+	opts := DefaultResolveOpts()
+	opts.OnSkipNonMarkdown = func(p string) { skipped = append(skipped, p) }
+
+	files, err := ResolveFilesWithOpts([]string{mdFile, txtFile, dir}, opts)
+	require.NoError(t, err)
+	require.Equal(t, []string{mdFile}, files)
+	assert.Equal(t, []string{txtFile}, skipped,
+		"only the explicitly named non-Markdown file should be reported")
 }
 
 func TestResolveFiles_Directory(t *testing.T) {