proposal: path/filepath: Deprecate Walk

[earthboundkid]

Proposal Details

filepath.WalkDir was added in Go 1.16. It's past time to go ahead and deprecate filepath.Walk, which is less efficient.

[gabyhelp]

Related Issues and Documentation

• path/filepath: add WalkDir (Walk using DirEntry) #42027 (closed)
• path/filepath: Walk behavior change from Go 1.1 #6679 (closed)
• Go 1 Release Notes > Minor changes to the library > The path/filepath package

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[robpike]

Define 'deprecate'.

[ianlancetaylor]

I'm not sure it's useful to deprecate a function that doesn't have an exact equivalent that people can simply switch to. The docs for filepath.Walk already say "Walk is less efficient than WalkDir, introduced in Go 1.16, which avoids calling os.Lstat on every visited file or directory."

[earthboundkid]

Define 'deprecate'.

Add // Deprecated: Use [WalkDir]. to the doc string, so my editor puts a squiggly line under accidental uses of Walk. I am aware that WalkDir exists and is more efficient but accidentally used Walk instead last week.

[earthboundkid]

I'm not sure it's useful to deprecate a function that doesn't have an exact equivalent that people can simply switch to. The docs for filepath.Walk already say "Walk is less efficient than WalkDir, introduced in Go 1.16, which avoids calling os.Lstat on every visited file or directory."

In most cases, the DirEntry is sufficient. If it’s not, you can just do a .FileInfo() call yourself at no loss of efficiency. Or just keep using Walk because it’s not going to be removed.

[ianlancetaylor]

Yes, you can update, but the update can't be easily done mechanically, and I don't think the performance advantage here is sufficient reason to push people to modify existing working code.

I think what you are looking for is something more like "hint when writing new code that a different choice would be better". Our current uses of deprecation apply to existing code as well. As far as I know we don't have that kind of hint at present.

[earthboundkid]

I don't think the expectation is that there's always a trivial find-and-replace for deprecations. For example, zip.FileHeader.ModifiedTime is a uint16 that is deprecated in favor of a time.Time Modified field. Presumably fixing that requires ripping out any code that uses the uint16 as a number.

My expectation is that a large percentage of the changes from Walk to WalkDir would just need to change from fs.FileInfo to fs.DirEntry in the callback, but the ones that need more than that are fairly easy to catch and fix when the type check fails. I think it actually could be done mechanically with a type aware rewrite tool.

[ianlancetaylor]

I'm not sure it's quite analogous. zip.FileHeader.ModifiedTime is deprecated because it doesn't have the same precision. Switching to Modified changes from 2-second precision to 1-second precision. So Modified is really better.

Walk and WalkDir are nearly the same. There isn't much of a reason to change existing code.

To be clear I certainly agree that new code should use WalkDir. I just don't think that deprecation as implemented today is necessarily the best path to make that happen.

[apocelipes]

Rather than marking a function as deprecated, I'd like "go vet" or gopls to give me some hints like "WalkDir performs better, you should use this" or something like that. And if you add this hint, the same thing should be applied to os.(*File).Readdir too.

[robpike]

The comments already say that. Go vet's guildelines (see the README) have no scope to do suggest faster code. Whether gopls's job includes repeating all the information in the comments is up to its authors. I don't believe it does, though.

[apparentlymart]

Reading the discussion so far, I agree that it seems like the most ideal answer here would be a new classification that is softer than "deprecated" but still machine-readable so that tools can react to it. However, I also have a feeling that discussing such a thing here is taking the issue off-topic. 🤔

At the risk of straying further off-topic 😬 ...

The idea of this being a gopls thing actually resonates with me because its code completion hints are very much relevant only when writing new code, and not when reading existing code. Is this issue describing a use-case for a way to declare that a particular symbol should be excluded from (or deprioritized in) code completion?

Of course that's not an answer for everyone, but I will admit that when I'm writing new code I do tend to "follow my nose" by selecting the most likely candidate from the code completion list, rather than carefully reviewing the documentation for each function. Having Walk either not appear at all, or always appear after WalkDir in the choice list, would make me less likely to select it.

[seankhliao]

I think as above, Deprecation is too strong for filepath.Walk.