Concise Magic File Names

Evolution Pitches
,
21

I strongly agree on the proposal of fixing this, but I'm a tentative -1 on the justifications for changing the behavior of #file:

However, a great deal of code already uses #file and would in practice probably never be converted to #fileName .

I agree with this, but I'm curious what proportion of projects actually use #file explicitly? I would guess that the vast majority of apps only use it indirectly by calling assert() or some other error handling or logging function that captures it as a default argument.

This magic behavior of assertions in the standard lib is also the reason why developers are inadvertently including path info in their binaries - because they aren't calling #file explicitly they don't realize it's being called at all. Those that are using it explicitly are much more likely to realize what it does and what the implications are.

So it seems that much of the benefit of changing #file would also be achieved by adding a #filename alternative and then replacing all uses of #file with #filename within the stdlib. (You could also reach out to popular 3rd party frameworks that use #file and make sure they are aware they should switch).

The "other alternatives" section also doesn't seem very exhaustive. For example here is another option that seems viable:

Deprecating (not removing) #file and then replacing it with #filename and #filepath seems like a good solution because then everyone using #file would get a warning in their project and would be encouraged to make an explicit choice to switch to the behavior they actually want, but it would not be a source-breaking change.

2 Likes
22

Thatā€™s actually the main alternative I discuss, and I agree that itā€™s the next-best answer.

23

I believe this to be a decisive point in favor of changing the behavior. The change is really on the level of a bugfix as far as I'm concerned. The existing behavior is wrong. (It's always bothered me.)

At the very least, if we decide it's absolutely necessary for #file to continue to evaluate to a path, it should be a relative path from the project root. But given Brent's description of how they are generated, it's not clear that is feasible.

3 Likes
24

@beccadax I think this would throw the whole proposal under the bus, but have you considered leaving #file as is and also not introducing any new variant similar to it but rather tackling a more commonly needed #context where we actually can implement the file differently as we would like?!

Edit: Ah sorry the was already mentioned upthread. Concise Magic File Names - #6 by sindresorhus

25

Ah sorry, I didn't see it under "Other alternatives" and somehow missed the main alternatives section above. In that case I withdraw my criticism, but I do think that this alternative is the better choice overall.

I'd also add that this option doesn't preclude eventually changing the behavior of #file, or renaming #fileName to #file after a sufficient deprecation period.

26

On the other hand:

Literal: #file. Type: String. Value: The name of the file in which it appears.

So this seems to be on the same level as changing the description or debugDescription of a type, or the unspecified order in which results in a sequence are produced, or other similar implementation details. These can all be source-breaking but only for undocumented/incorrect usage.

4 Likes
27

I understand this change might not come under the official "source compatibility" story but the reality is that it will break significant amount of existing code. Sure it would be easy to fix or migrate that code but it's still a lot of churn. IMO it'll be better if users can opt-into the new behavior using a new language version or some other mechanism.

28

Can you provide any concrete examples? Others have posited that it may break testing code, but no examples have been provided.

29

In SwiftPM itself:

But it is only testing code, since it requires the presence of the source repository. And it is trivial to switch to #filePath.

30

It will break hundreds of tests Iā€™ve written in the same manner, but I still consider the churn worth it for benefits it would automatically bring to the security and size of the actual product binaries.

4 Likes
31

I spent a couple hours this afternoon trying to gather more data on how bad these breaks will be.

Although Swift CI doesn't actually use this feature, the Source Compatibility Suite is capable of running tests for projects that tell it how to run their test targets. 29 projects have included test-running information in their configuration.

I've now merged the proposed change behind a compiler flag (-enable-experimental-concise-pound-file), so I ran tests on those 29 projects with and without the flag. A bunch of projects failed in both casesā€”only 21 of the 29 passed in at least one language version modeā€”but three projects (14%) failed only with -enable-experimental-concise-pound-file:

  • Html
  • Lark
  • swift-nio-http2

All three of these projects fail because they tried to find test fixtures relative to #file; the projects otherwise appear to work correctly. All three test suites produced failure messages suggesting some kind of trouble loading fixtures:

HtmlSnapshotTestingTests.swift (HtmlSnapshotTestingTests):421: error: -[HtmlSnapshotTestingTests.SnapshotTestingTests testComplexHtml] : failed - No reference was found on disk. Automatically recorded snapshot: ā€¦

<unknown>:0: error: -[SchemaParserTests.WebServiceDescriptionTests testImport] : failed: caught error: The file ā€œimport.wsdlā€ couldnā€™t be opened because there is no such file.

HuffmanCodingTests.swift (NIOHPACKTests):177: error: -[NIOHPACKTests.HuffmanCodingTests testComplexDecodingPerformance] : failed - Couldn't load fixture data

When I examined these projects, I had no trouble finding the places this was done and it looked like they could be switched to #filePath very easily.

(Interestingly, two projects wrote URL(fileURLForPath: #file) exactly; the third wrapped the #file in String(...) first. Maybe we could add a warning for those patterns?)

Conclusions:

  • Some projects will break.
  • They will break in obvious ways.
  • They won't be hard to fix.
9 Likes
32

I am not really worried about the migration story. My only concern is the fact that you need to migrate in order to use newer tools. For example, I write a lot of tooling using Swift packages. I barely ever need to change or update those packages but such a change would mean that now I have to go update most of them when the new compiler is released. It's not the end of the world but it'll be annoying and time consuming. It seems to me that this problem can be easily avoided by allowing users to opt-into the new behavior.

1 Like
33

IMO, anyone depending on #file for non-logging purposes is already relying on weakly-defined (and I hesitate to use the word "defined" here) behavior, and behavior which changes depending on nothing other than which build system you use (!).

To illustrate, consider this unit test from swift-tools-support-core, which passes #file to an AbsolutePath initializer that traps if the path string it's given is not absolute.

That test succeeds when built with SwiftPM or Xcode, because the those tools pass the files to the compiler as absolute paths. But when I recently tried to wire up Bazel test targets for that package, those same tests fail as written because Bazel does not/cannot pass absolute paths.

Even if this test was migrated to the proposed #filePath, it would still be broken when built with Bazel because its runtime behavior depends on the format of the inputs passed to the compiler. Assuming that #file will always start with "/" is relying on a side-effect of the common behavior of Xcode and SwiftPM, but not behavior guaranteed by the Swift compiler or language.

Similarly, anyone using #file to co-locate test fixtures is already hardcoding one of two fragile assumptions, either:

  1. the path they get from #file will be absolute, or
  2. it won't be, and they will be required to run the test with CWD equal to the package/project root so that the files will be found via their relative paths.

Given the fragility of this behavior, and the security/leak concerns raised elsewhere in the pitch/thread, I think there are sufficient reasons for this kind of use of #file to be abandoned even without considering the migration that this proposal would require, and to keep the sole recommended use case of #filePath as a diagnostic tool for libraries like XCTest that need to provide more precise information about the path to a source file where an assertion occurred. (And then the test runner, be it Xcode or a component in some other build system can do the right thing with that output based on whether it passes those paths absolute or relative.)

11 Likes
34

I really think this is the best way forward. These things like #file, #line and #function are such awful holdovers from C. While the information is necessary, the way they are presented does not fit well with Swift.

Some people have have already touched on the shortfalls - for example, there is literally no reliable way in Swift to get your current module name. I had a need for this a couple of years ago (something to do with NSClassFromString IIRC), and I was shocked to find there was no answer. C doesn't have modules, so they never needed this, but modules are a core component of Swift (for one thing, file and type-names must be unique within a module, but can be the same across modules). If we're going to start making subtle, breaking changes to these values, why not just replace them with something that is truly made for Swift?

Another shortfall - these things are not defined in the standard library. Where is the documentation for #file, which defines the specific guarantees it makes and the things you can use this value for? Oh, it doesn't exist. I would be in favour of a Context or SourceLocation type in the standard library where such things could be documented. For example - is #column Unicode-aware?

I agree that full paths in #file is a problem, and we should fix it, but I don't think it's urgent enough to make this small fix worthwhile, especially since it will likely break existing code. This little corner of the language needs sprucing up anyway, and now we have an important lesson to use when deciding how to do that.

3 Likes
35

+1 on this proposal from me, on the strength of the security issues alone.

I don't find the source-breaking concerns that compelling, since they're relying on essentially undocumented behavior.

2 Likes
36

What is your proposal for how to locate test fixtures, if it isn't "use #filePath" or "use #file"?

37

I used to use #file as the base path to find test ressources. Now I use:

Bundle.init(for: MyTestClass.self).url(forResource:ofType:)

in a test bundle because it is more portable. Now my tests can run on iOS and inside a sandboxed process.

1 Like
38

Use the SwiftPM resources feature: SE-0271

39

For this to work in the SwiftPM case requires the SwiftPM resources feature outlined by @Karl.

Ah, thanks for reminding me of this change. That's sufficient for my case I think.

40

I agree with the above postersā€”SE-0271 was mentioned in @beccadax's write-up and should be sufficient for just about all cases where test fixtures are needed at runtime.

Treating these as runtime resources and using appropriate platform APIs, rather than making assumptions about source tree structure and compiler behavior, is the right solution. Current tricks like #file don't scale; reproducibility/caching problems abound for distributed builds, and it forces too many assumptions about the runtime environment on those tests when there are more general alternatives. Before SwiftPM supported resources there weren't really better options, but now that there are, we shouldn't let the old methods block fixing really bad behavior.

5 Likes