Skip to content

Commit 9f90538

Browse files
committed
PR feedback
1 parent 26d66cd commit 9f90538

5 files changed

Lines changed: 173 additions & 6 deletions

File tree

.github/workflows/ci-docs.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,19 @@ jobs:
247247
path: code-fence-output.txt
248248
retention-days: 7
249249

250+
# ============================================================================
251+
# DOC CLAIMS - Check doc comments for misleading claims
252+
# ============================================================================
253+
doc-claims:
254+
name: Doc Claims Check
255+
runs-on: ubuntu-latest
256+
257+
steps:
258+
- uses: actions/checkout@v6
259+
260+
- name: Check doc comment accuracy
261+
run: ./scripts/ci/check-doc-claims.sh
262+
250263
link-check:
251264
name: Link Check
252265
runs-on: ubuntu-latest

.pre-commit-config.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,13 @@ repos:
102102
pass_filenames: false
103103
files: '\.rs$'
104104

105+
- id: check-doc-claims
106+
name: check doc comment accuracy
107+
entry: bash scripts/ci/check-doc-claims.sh
108+
language: system
109+
pass_filenames: false
110+
files: '\.rs$'
111+
105112
# ══════════════════════════════════════════════════════════════════════
106113
# Documentation checks
107114
# ══════════════════════════════════════════════════════════════════════

scripts/ci/check-doc-claims.sh

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
#!/bin/bash
2+
# Doc Comment Accuracy Check for Fortress Rollback
3+
#
4+
# Checks that doc comments mentioning "downcast" are backed by actual
5+
# downcasting infrastructure in the same file. This prevents misleading
6+
# documentation that references capabilities the code doesn't support.
7+
#
8+
# Usage: ./scripts/ci/check-doc-claims.sh [options]
9+
# ./scripts/ci/check-doc-claims.sh # Check all Rust files
10+
# ./scripts/ci/check-doc-claims.sh --verbose # Show all files checked
11+
# ./scripts/ci/check-doc-claims.sh --help # Show help
12+
#
13+
# Exit codes:
14+
# 0 - No issues found
15+
# 1 - Misleading doc comments detected
16+
17+
set -euo pipefail
18+
19+
# Configuration
20+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
21+
PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
22+
23+
# Colors
24+
RED='\033[0;31m'
25+
GREEN='\033[0;32m'
26+
YELLOW='\033[1;33m'
27+
BLUE='\033[0;34m'
28+
NC='\033[0m' # No Color
29+
30+
# Options
31+
VERBOSE=false
32+
33+
print_usage() {
34+
echo "Usage: $0 [options]"
35+
echo ""
36+
echo "Options:"
37+
echo " --verbose Show all files checked"
38+
echo " --help Show this help message"
39+
echo ""
40+
echo "Checks doc comments for claims about downcasting that aren't"
41+
echo "backed by actual downcasting infrastructure in the same file."
42+
}
43+
44+
main() {
45+
# Parse arguments
46+
while [[ $# -gt 0 ]]; do
47+
case "$1" in
48+
--verbose)
49+
VERBOSE=true
50+
shift
51+
;;
52+
--help)
53+
print_usage
54+
exit 0
55+
;;
56+
*)
57+
echo "Unknown argument: $1"
58+
print_usage
59+
exit 1
60+
;;
61+
esac
62+
done
63+
64+
echo "=========================================="
65+
echo " Doc Comment Accuracy Check"
66+
echo "=========================================="
67+
echo ""
68+
69+
# Patterns that indicate actual downcasting infrastructure
70+
# If a file mentions downcasting in docs, it should contain at least one of these
71+
local downcast_infra_patterns='(as_any|downcast_ref|downcast_mut|dyn Any|: Any|impl Any|Any \+|Any\+|\.downcast\b)'
72+
73+
local issues=0
74+
local files_with_claims=0
75+
76+
# Find all Rust source files (excluding target directories)
77+
while IFS= read -r file; do
78+
[[ -z "$file" ]] && continue
79+
80+
local rel_path="${file#"$PROJECT_ROOT/"}"
81+
82+
# Find doc comment lines mentioning "downcast" (case-insensitive)
83+
local doc_matches
84+
doc_matches=$(grep -niE '^\s*///.*downcast|^\s*//!.*downcast' "$file" 2>/dev/null || true)
85+
86+
if [[ -z "$doc_matches" ]]; then
87+
if [[ "$VERBOSE" == "true" ]]; then
88+
echo -e " ${GREEN}OK${NC}: $rel_path (no downcast doc claims)"
89+
fi
90+
continue
91+
fi
92+
93+
files_with_claims=$((files_with_claims + 1))
94+
95+
if [[ "$VERBOSE" == "true" ]]; then
96+
echo -e " ${YELLOW}Checking${NC}: $rel_path (has downcast doc claims)"
97+
fi
98+
99+
# Check if the file has actual downcasting infrastructure
100+
local has_infra
101+
has_infra=0
102+
has_infra=$(grep -cE "$downcast_infra_patterns" "$file" 2>/dev/null) || true
103+
104+
if [[ "$has_infra" -eq 0 ]]; then
105+
issues=$((issues + 1))
106+
echo ""
107+
echo -e "${RED}ERROR${NC}: $rel_path"
108+
echo -e " Doc comments mention \"downcast\" but no downcasting infrastructure found."
109+
echo -e " ${YELLOW}Doc comment(s):${NC}"
110+
while IFS= read -r match_line; do
111+
echo -e " $match_line"
112+
done <<< "$doc_matches"
113+
echo -e " ${BLUE}Expected one of:${NC} as_any, downcast_ref, downcast_mut, dyn Any, : Any"
114+
echo -e " ${BLUE}Fix:${NC} Either add downcasting support or update the doc comment"
115+
echo -e " to accurately describe the actual pattern used."
116+
else
117+
if [[ "$VERBOSE" == "true" ]]; then
118+
echo -e " ${GREEN}OK${NC}: downcasting infrastructure found ($has_infra occurrence(s))"
119+
fi
120+
fi
121+
122+
done < <(find "$PROJECT_ROOT/src" "$PROJECT_ROOT/tests" "$PROJECT_ROOT/examples" "$PROJECT_ROOT/benches" \
123+
-name '*.rs' -print 2>/dev/null \
124+
| sort)
125+
126+
echo ""
127+
128+
if [[ "$issues" -eq 0 ]]; then
129+
echo -e "${GREEN}SUCCESS: No misleading downcast doc claims found.${NC}"
130+
if [[ "$files_with_claims" -gt 0 ]]; then
131+
echo -e " ($files_with_claims file(s) with downcast references verified)"
132+
fi
133+
exit 0
134+
fi
135+
136+
echo -e "${RED}FAILED: $issues file(s) have misleading downcast doc claims.${NC}"
137+
echo ""
138+
echo "Doc comments should accurately describe the code's capabilities."
139+
echo "If downcasting isn't supported, describe the actual pattern instead."
140+
exit 1
141+
}
142+
143+
main "$@"

src/error.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,8 @@ pub enum RleDecodeReason {
245245
/// An unknown or unexpected error occurred during RLE decoding.
246246
///
247247
/// This variant is used as a fallback when the underlying error cannot be
248-
/// mapped to a more specific reason (e.g., when downcasting fails).
248+
/// mapped to a more specific reason (e.g., when a `FortressError` variant
249+
/// does not match the expected structured RLE error kind).
249250
Unknown,
250251
}
251252

@@ -306,8 +307,8 @@ pub enum DeltaDecodeReason {
306307
},
307308
/// An unknown or unexpected error occurred during delta decoding.
308309
///
309-
/// This variant is used as a fallback when the underlying error cannot be
310-
/// mapped to a more specific reason (e.g., when downcasting fails).
310+
/// This variant exists for forward compatibility and as a fallback
311+
/// if future error-mapping code encounters an unexpected error kind.
311312
Unknown,
312313
}
313314

src/sessions/p2p_session.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,10 +1309,13 @@ impl<T: Config> P2PSession<T> {
13091309

13101310
/// Returns a reference to the telemetry observer, if one is attached.
13111311
///
1312-
/// This is useful for inspecting the attached telemetry observer in tests,
1313-
/// e.g., by downcasting to [`CollectingTelemetry`].
1312+
/// In tests, the typical pattern is to keep a separate
1313+
/// <code>Arc<[CollectingTelemetry]></code> clone and call methods on it directly,
1314+
/// rather than going through this accessor. This method is primarily useful
1315+
/// when you need to confirm that a telemetry observer is attached or to pass
1316+
/// the trait object to other code.
13141317
///
1315-
/// [`CollectingTelemetry`]: crate::telemetry::CollectingTelemetry
1318+
/// [CollectingTelemetry]: crate::telemetry::CollectingTelemetry
13161319
#[must_use]
13171320
pub fn telemetry(&self) -> Option<&Arc<dyn SessionTelemetry>> {
13181321
self.telemetry.as_ref()

0 commit comments

Comments
 (0)