-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Populate the "Possible Values" section based on OAS data #857
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
This source file is part of the Swift.org open source project | ||
|
||
Copyright (c) 2024 Apple Inc. and the Swift project authors | ||
Licensed under Apache License v2.0 with Runtime Library Exception | ||
|
||
See https://swift.org/LICENSE.txt for license information | ||
See https://swift.org/CONTRIBUTORS.txt for Swift project authors | ||
*/ | ||
|
||
import Foundation | ||
|
||
/// Translates a symbol's possible values into a render node's Possible Values section. | ||
struct PossibleValuesSectionTranslator: RenderSectionTranslator { | ||
func translateSection( | ||
for symbol: Symbol, | ||
renderNode: inout RenderNode, | ||
renderNodeTranslator: inout RenderNodeTranslator | ||
) -> VariantCollection<CodableContentSection?>? { | ||
translateSectionToVariantCollection( | ||
documentationDataVariants: symbol.possibleValuesSectionVariants | ||
) { _, possibleValuesSection in | ||
// Render section only if values were listed in the markdown | ||
// and there are value defined in the symbol graph. | ||
guard !possibleValuesSection.documentedValues.isEmpty else { return nil } | ||
guard !possibleValuesSection.definedValues.isEmpty else { return nil } | ||
|
||
// Build a lookup table of the documented values | ||
var documentationLookup = [String: PossibleValue]() | ||
possibleValuesSection.documentedValues.forEach { documentationLookup[$0.value] = $0 } | ||
|
||
// Generate list of possible values for rendering from the full list of defined values, | ||
// pulling in any documentation from the documented values list when available. | ||
let renderedValues = possibleValuesSection.definedValues.map { | ||
let valueString = String($0) | ||
let possibleValue = documentationLookup[valueString] ?? PossibleValue(value: valueString, contents: []) | ||
let valueContent = renderNodeTranslator.visitMarkupContainer( | ||
MarkupContainer(possibleValue.contents) | ||
) as! [RenderBlockContent] | ||
return PossibleValuesRenderSection.NamedValue(name: possibleValue.value, content: valueContent) | ||
} | ||
|
||
return PossibleValuesRenderSection( | ||
title: PossibleValuesSection.title, | ||
values: renderedValues | ||
) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
This source file is part of the Swift.org open source project | ||
|
||
Copyright (c) 2024 Apple Inc. and the Swift project authors | ||
Licensed under Apache License v2.0 with Runtime Library Exception | ||
|
||
See https://swift.org/LICENSE.txt for license information | ||
See https://swift.org/CONTRIBUTORS.txt for Swift project authors | ||
*/ | ||
|
||
import SymbolKit | ||
|
||
/// A section that contains an type's possible values. | ||
public struct PossibleValuesSection { | ||
public static var title: String { | ||
return "Possible Values" | ||
} | ||
|
||
/// The list of possible values. | ||
public var documentedValues: [PossibleValue] | ||
|
||
/// The list of defined values for the symbol. | ||
public var definedValues: [SymbolGraph.AnyScalar] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/* | ||
This source file is part of the Swift.org open source project | ||
|
||
Copyright (c) 2023 Apple Inc. and the Swift project authors | ||
Licensed under Apache License v2.0 with Runtime Library Exception | ||
|
||
See https://swift.org/LICENSE.txt for license information | ||
See https://swift.org/CONTRIBUTORS.txt for Swift project authors | ||
*/ | ||
|
||
import Markdown | ||
import SymbolKit | ||
|
||
/// Documentation about the possible values for a symbol. | ||
public struct PossibleValue { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems only relevant within |
||
/// The string representation of the value. | ||
public var value: String | ||
/// The content that describe the value. | ||
public var contents: [Markup] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,7 @@ let simpleListItemTags = [ | |
] | ||
|
||
extension Sequence where Element == InlineMarkup { | ||
private func splitNameAndContent() -> (name: String, nameRange: SourceRange?, content: [Markup], range: SourceRange?)? { | ||
private func splitNameAndContent(skipStartingTag: Bool = false) -> (name: String, nameRange: SourceRange?, content: [Markup], range: SourceRange?)? { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why is this parameter needed? If the code scans up to the first colon then does it matter if the line looks like
or
? |
||
var iterator = makeIterator() | ||
guard let initialTextNode = iterator.next() as? Text else { | ||
return nil | ||
|
@@ -59,8 +59,16 @@ extension Sequence where Element == InlineMarkup { | |
guard let colonIndex = initialText.firstIndex(of: ":") else { | ||
return nil | ||
} | ||
|
||
let nameStartIndex = initialText[...colonIndex].lastIndex(of: " ").map { initialText.index(after: $0) } ?? initialText.startIndex | ||
|
||
let nameStartIndex: String.Index | ||
if skipStartingTag == true, var spaceIndex = initialText[...colonIndex].firstIndex(of: " ") { | ||
while initialText[spaceIndex] == " " { | ||
spaceIndex = initialText.index(spaceIndex, offsetBy: 1) | ||
} | ||
Comment on lines
+65
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directly manipulating a string index and using it to access the content in a loop like this is a bad way of working with Strings in Swift. It's possibly accidentally quadratic because the characters are not known to be the same number of bytes. I think this is trying to do something like the below to find the start of the next if there are multiple spaces between the colon and the word let nameStartIndex = initialText[...colonIndex].drop(while: { $0 != " " }).dropFirst().firstIndex(where: { $0 != " " })
?? initialText.startIndex in which case it might be more robust to use let nameStartIndex = initialText[...colonIndex].drop(while: { !$0.isWhitespace }).dropFirst().firstIndex(where: { !$0.isWhitespace })
?? initialText.startIndex Regardless. It would be good to add a short comment to describe why this is happening. |
||
nameStartIndex = spaceIndex | ||
} else { | ||
nameStartIndex = initialText.startIndex | ||
} | ||
let parameterName = initialText[nameStartIndex..<colonIndex] | ||
guard !parameterName.isEmpty else { | ||
return nil | ||
|
@@ -93,7 +101,7 @@ extension Sequence where Element == InlineMarkup { | |
} | ||
|
||
func extractParameter(standalone: Bool) -> Parameter? { | ||
if let (name, nameRange, content, itemRange) = splitNameAndContent() { | ||
if let (name, nameRange, content, itemRange) = splitNameAndContent(skipStartingTag: standalone == true) { | ||
return Parameter(name: name, nameRange: nameRange, contents: content, range: itemRange, isStandalone: standalone) | ||
} | ||
return nil | ||
|
@@ -106,6 +114,13 @@ extension Sequence where Element == InlineMarkup { | |
return nil | ||
} | ||
|
||
func extractPossibleValue() -> PossibleValue? { | ||
if let (value, _, content, _) = splitNameAndContent() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New code should capture and use the It would also be good to add a test that verifies that diagnostics in the possible value content are presented with the relevant source and range information. |
||
return PossibleValue(value: value, contents: content) | ||
} | ||
return nil | ||
} | ||
|
||
func extractHTTPParameter() -> HTTPParameter? { | ||
if let (name, _, content, _) = splitNameAndContent() { | ||
return HTTPParameter(name: name, source: nil, contents: content) | ||
|
@@ -237,6 +252,65 @@ extension ListItem { | |
return dictionaryKeys | ||
} | ||
|
||
/** | ||
Extract a standalone possible value description from this list item. | ||
|
||
Expected form: | ||
|
||
```markdown | ||
- possibleValue x: The meaning of x | ||
``` | ||
*/ | ||
func extractStandalonePossibleValue() -> PossibleValue? { | ||
guard let remainder = extractTag(TaggedListItemExtractor.possibleValueTag) else { | ||
return nil | ||
} | ||
return remainder.extractPossibleValue() | ||
} | ||
|
||
/** | ||
Extracts an outline of possible values from a sublist underneath this list item. | ||
|
||
Expected form: | ||
|
||
```markdown | ||
- PossibleValues: | ||
- x: Meaning of x | ||
- y: Meaning of y | ||
``` | ||
|
||
> Warning: Content underneath `- PossibleValues` that doesn't match this form will be dropped. | ||
*/ | ||
func extractPossibleValueOutline() -> [PossibleValue]? { | ||
guard extractTag(TaggedListItemExtractor.possibleValuesTag + ":") != nil else { | ||
return nil | ||
} | ||
|
||
var possibleValues = [PossibleValue]() | ||
|
||
for child in children { | ||
// The list `- PossibleValues:` should have one child, a list of values. | ||
guard let possibleValuesList = child as? UnorderedList else { | ||
// If it's not, that content is dropped. | ||
continue | ||
} | ||
|
||
// Those sublist items are assumed to be a valid `- ___: ...` possible value form or else they are dropped. | ||
for child in possibleValuesList.children { | ||
guard let listItem = child as? ListItem, | ||
let firstParagraph = listItem.child(at: 0) as? Paragraph, | ||
let possibleValue = Array(firstParagraph.inlineChildren).extractPossibleValue() else { | ||
continue | ||
} | ||
// Don't forget the rest of the content under this possible value list item. | ||
let contents = possibleValue.contents + Array(listItem.children.dropFirst(1)) | ||
|
||
possibleValues.append(PossibleValue(value: possibleValue.value, contents: contents)) | ||
} | ||
} | ||
return possibleValues | ||
} | ||
|
||
/** | ||
Extract a standalone HTTP parameter description from this list item. | ||
|
||
|
@@ -530,6 +604,8 @@ struct TaggedListItemExtractor: MarkupRewriter { | |
static let parametersTag = "parameters" | ||
static let dictionaryKeyTag = "dictionarykey" | ||
static let dictionaryKeysTag = "dictionarykeys" | ||
static let possibleValueTag = "possiblevalue" | ||
static let possibleValuesTag = "possiblevalues" | ||
|
||
static let httpBodyTag = "httpbody" | ||
static let httpResponseTag = "httpresponse" | ||
|
@@ -540,6 +616,7 @@ struct TaggedListItemExtractor: MarkupRewriter { | |
static let httpBodyParametersTag = "httpbodyparameters" | ||
|
||
var parameters = [Parameter]() | ||
var possibleValues = [PossibleValue]() | ||
var dictionaryKeys = [DictionaryKey]() | ||
var httpResponses = [HTTPResponse]() | ||
var httpParameters = [HTTPParameter]() | ||
|
@@ -643,6 +720,16 @@ struct TaggedListItemExtractor: MarkupRewriter { | |
// - dictionaryKey x: ... | ||
dictionaryKeys.append(dictionaryKeyDescription) | ||
return nil | ||
} else if let possibleValueDescription = listItem.extractPossibleValueOutline() { | ||
// - PossibleValues: | ||
// - x: ... | ||
// - y: ... | ||
possibleValues.append(contentsOf: possibleValueDescription) | ||
return nil | ||
} else if let possibleValueDescription = listItem.extractStandalonePossibleValue() { | ||
// - possibleValue x: ... | ||
possibleValues.append(possibleValueDescription) | ||
return nil | ||
} else if let httpParameterDescription = listItem.extractHTTPParameterOutline() { | ||
// - HTTPParameters: | ||
// - x: ... | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the structure of this type. Is there any relation between the
documentedValues
anddefinedValues
? It looks to me like each documented values'svalue
may(?) correspond to one of the defined values? If that's correct, would it be better to model this like[SymbolGraph.AnyScalar: [Markup]?]
or something along those lines?It's hard and takes a long time to change a public API after it's been added. If we aren't sure that we have the right API then we shouldn't make it public yet.