Increase namespace/type-name shortening threshold from 64 to 200#7914
Conversation
Reduce the aggressiveness of the recently-added namespace segment and type name shortening by raising the default length threshold from 64 to 128 characters. Names between 65 and 128 characters now keep their full names instead of being truncated with a hash suffix. - StringExtensions.DefaultMaxNameSegmentLength: 64 -> 128 - CommonLanguageRefiner.ShortenOversizedNamespaceSegments default: 64 -> 128 - Updated affected tests and CHANGELOG Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Code Coverage OverviewLanguages: C# C# / code-coverage/dotnetThe overall coverage in the branch is 72%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
Cover the shortening behavior for Python (which inherits the 128-char default) mirroring the Java/PHP tests, plus a test asserting that names between the old (64) and new (128) thresholds are left intact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jingjingjia-ms
left a comment
There was a problem hiding this comment.
Please correct me if I'm wrong: with the per-segment limit raised to 128, I think it can actually reintroduce the original problem #7713 was filed to fix.
Taking the deviceReport example from that issue — the directory segment (237 chars) and class name (251 chars) are both still over 128, so they do get trimmed, but only down to 126 chars each. Once you combine them with the namespace prefix and separators, the full path becomes:
D:\a\_work\1\s\kiota\output\com\microsoft\graph\beta\networkaccess\reports\microsoftgraphnetworkaccessdevicereportwithstartdatetimewithenddatetimediscoveredapplicationsegmentiddiscoveredapplicat_<8hex>\MicrosoftGraphNetworkaccessDeviceReportWithStartDateTimeWithEndDateTimediscoveredApplicationSegmentIdDiscoveredApplicat_<8hex>RequestBuilder.java
= 333 chars, back over the Windows MAX_PATH limit.
|
TLDR; The limit is per-segment / per-name, not on the total path length. Looking at the two decision points: Namespace segments (line 1647):
The full namespace name is split on Type names (in ShortenCodeElementNameIfOversized , line ~1687):
Each class/enum/interface name is checked on its own against the same 64-char threshold. What this means• There is no accounting for total path length (root path + package dirs + all segments + filename). The fix caps each component to a bounded size and relies on the fact that bounding each component keeps the total comfortably under Windows MAX_PATH. Limits we need to be aware ofAssuming that the OS has LongPathsEnabled
|
|
Correct me if im wrong, but at least for java this would result in breaking changes, as the file name must match the class name. A related concern is that for Java we actually encourage users to explicitly hardcode certain namespaces to disambiguate. Applying this fix across the board to namespaces also creates a possible breaking change. |
|
The fileName == className concern is already managed in the previous work. this fix should be adjusted so that we only hit the cases where we were unable to produce the SDK due to truncation so that we don't introduce a v1 breaking change. |
|
Sounds good, will increase our limit so our maximum output is 255 to reduce possible changes. Issue with namespace changes being impacted is unlikely as we currently do not have any folders reaching the limit so they will not be truncated hence any hardcoded namespaces should not be affected. |
Update the threshold to 255 characters (the common file-system per-component limit) so only names longer than 255 are shortened. When shortened the output is capped at exactly 255 chars: a 246-char prefix, an underscore, and the existing 8-char hash suffix. - StringExtensions.DefaultMaxNameSegmentLength: 128 -> 255 - CommonLanguageRefiner.ShortenOversizedNamespaceSegments default: 128 -> 255 - Updated affected tests (inputs lengthened past 255) and CHANGELOG Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tening-threshold' into ramsessanchez/increase-name-shortening-threshold # Conflicts: # CHANGELOG.md
Drop DefaultMaxNameSegmentLength / ShortenOversizedNamespaceSegments default from 255 to 250 so the shortened type name plus the extension appended by path segmenters (e.g. .java, .php) stays within the 255 file-system per-component (NAME_MAX) limit. Update tests and CHANGELOG. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Python's path segmenter converts CodeDOM names to snake_case, adding an underscore per capital letter after refiner-level shortening runs, which can push a 250-char name past the 255 file-system limit once ".py" is appended. Pass maxSegmentLength: 240 from PythonRefiner to leave headroom. Update Python tests and CHANGELOG accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the unified DefaultMaxNameSegmentLength / ShortenOversizedNamespaceSegments default to 200 and remove the Python-specific override. 200 keeps shortened names well below the 255 file-system per-component limit, leaving room for extensions and language-specific suffixes (e.g. Java nested-class ".class" files like "$GetQueryParameters.class" and the extra underscores Python adds when converting names to snake_case). Update tests and CHANGELOG. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…er action The kiota-dom-export-diff-tool is a Docker container action that mounts the repository at /github/workspace. The workflow was passing it the absolute host path to the patch file, which does not exist inside the container, causing a DirectoryNotFoundException whenever a non-empty patch was produced (e.g. the Java job). Output the workspace-relative file name instead so the container resolves it against its mounted workdir. The C# job was unaffected only because its patch is empty and the diff step is skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Reduces the sensitivity of the recently-added namespace segment and type name shortening (PRs #7714, #7901) by raising the default length threshold from 64 to 255 characters. Names between 65 and 255 characters now keep their full names instead of being truncated and given a hash suffix.
Changes
StringExtensions.DefaultMaxNameSegmentLength: 64 → 255 (+ doc)CommonLanguageRefiner.ShortenOversizedNamespaceSegmentsdefaultmaxSegmentLength: 64 → 255 (Java/Python/PHP/TypeScript refiners inherit the new default)StringExtensionsTests,JavaLanguageRefinerTests,PhpLanguageRefinerTests,TypeScriptLanguageRefinerTests) — assertion bounds and inputs adjusted so shortening is still exercised at the new thresholdOut of scope
The file-name path shortening in the C#/Go/Ruby PathSegmenters is unchanged.
Testing
Targeted
dotnet testover the 4 affected test classes: 116/116 passed, no new warnings/errors.