[heckj]

Summary

fixes: #1704

A page with no availability information is incorrectly reported as deprecated. I think this is a regression from #1686.

Availability.isDeprecated uses allSatisfy over the platforms, and every platform starts as .unavailable. If nothing ever adds information, allSatisfy is vacuously true. The page's topic render reference gets "deprecated": true. DocC-Render then strikes it through and adds a "Deprecated" badge wherever it's curated.

Modules are the common case. A module's availability only comes from the Info.plist defaults. A module without defaults gets no other information.

isDeprecated now returns false when no platform, known or custom, has any availability information. A platform that is explicitly unavailable still counts as information, so symbols that are obsoleted or unavailable everywhere keep their current behavior.

Dependencies

None.

Testing

Steps:

1. Check out the first commit (the test only) and run swift test --filter moduleWithoutAvailabilityInformationIsNotConsideredDeprecated. It fails because the module and its symbol are reported deprecated.
2. Check out the second commit (the fix) and run the same command. It passes.
3. To see it end to end, run docc convert on a catalog with one module symbol graph and no Info.plist availability. Then check references in the module's render JSON for a deprecated flag. The steps are in deprecated content regression in docc-main #1704.

Checklist

• Added tests
• Ran the ./bin/test script and it succeeded. I ran swift test (all Swift Testing and XCTest suites pass), but not the full script.
• Updated documentation if necessary. None needed.

[d-ronnqvist]

Thanks for finding and fixing this issue.
However, I'd prefer to fix this bug with how the module's availability is created so that it's the same as for all other symbols.

[d-ronnqvist]

I think that I'd prefer to fix this by populating the module's availability instead so that it behaves the same as all the other symbols.

If you replace https://github.com/swiftlang/swift-docc/blob/main/Sources/SwiftDocC/Infrastructure/DocumentationContext.swift#L1315-L1322 with

var symbolBaseAvailability = Availability(defaultAvailability: inputs.info.defaultAvailability?.modules[moduleName])
symbolBaseAvailability.markAllEncounteredPlatforms(in: unifiedGraph)

documentationCache.reference(symbolID: moduleName).map { moduleReference in
    var moduleAvailability = symbolBaseAvailability
    if let moduleSymbol = documentationCache[moduleReference]?.unifiedSymbol {
        for language in moduleSymbol.sourceLanguages {
            moduleAvailability.addInSourceAvailability(from: moduleSymbol, matchingLanguage: language)
        }
    }
    moduleAvailability.finalizePlatformFallbacks()
    (documentationCache[moduleReference]?.semantic as! Symbol).consolidatedAvailability = .init(defaultVariantValue: moduleAvailability)
}

the new test passes for me

[heckj]

totally works for me, thank you! Let me circle that up and replace this PR's implementation. Any strong preference on breaking out that repeatable validation logic? Happy to do the lifting to make the tests better structured - slapped this out primarily to just show the issue and work it.

[d-ronnqvist]

Do you mean sharing it for those two callers in that one test or are you thinking that there would additional callers in other tests?

[heckj]

no strong opinion here - was more just looking for high level guidance for when you thought it was worthwhile to break out a validation bit like we did here.

[heckj]

I applied your path and it brought up a complication in an existing test. Kinda need your input on the follow up, as I wasn't sure which way made more sense and don't know all the implications.

When there's an Info.plist default with no version, this would overwrite that availability.
testInheritDefaultAvailabilityOptions (lines 532–533) gets complicated

Setup: the Info.plist declares iOS with no version, and the symbol graph is for iOS.

• On main, the module page shows ["iOS"].
• With this change, the module page shows no platforms (metadata.platforms == nil).

My read of the flow that gets us here:

1. The Info.plist default gives iOS the source .defaultFromPropertyList.
2. addInSourceAvailability finds the module in the iOS graph and raises the source to .foundInSymbolGraph. The state stays "available, no version".
3. makePlatforms (the guard at Availability.swift:367) won't display a list that has only versionless .foundInSymbolGraph platforms.

A symbol with no in-source availability goes through the same steps and shows no platforms. With that, the module matches symbols, but it loses the Info.plist default.

Which behavior do we want?

• The module should match the symbols, so update the test to expect nil.
• The module should show the versionless Info.plist default. That implies that the in-source pass shouldn't overwrite a .defaultFromPropertyList source. The symbol path shares that code, so this would change symbols too.

[d-ronnqvist]

Which behavior do we want?

• The module should match the symbols, so update the test to expect nil.
• The module should show the versionless Info.plist default. That implies that the in-source pass shouldn't overwrite a .defaultFromPropertyList source. The symbol path shares that code, so this would change symbols too.

That's an annoying inconsistency in behavior between the module's page and other symbol pages that I had previously missed.

In that case it might be better to start out with an isEmpty check in Availability/isDeprecated which we can fix later.

What's important is that we capture both behaviors as new tests in AvailabilityTests:

• one test that a module with no availability information isn't deprecated (which you already have)
• one test that a module with some versionless default availability displays that information but the symbols in that module doesn't display it. I filed rdar://190715484 which would be good to refer to in a comment in that test.