Update ClangFormat configuration file for ClangFormat 14.0.0 #254
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
per1234
added
type: enhancement
|
The validation check is expected to fail? |
|
Hi @umbynos. Yes, it is expected. Please see the explanation in the "Schema validation failure" section of the PR message. |
|
The fix for the error in the |
umbynos
reviewed
Aug 1, 2022
umbynos
approved these changes
Aug 1, 2022
The output from the `clang-format --dump-config` command is compared against Arduino's configuration file to check for additions, removals, or type changes resulting from updating to a new version of ClangFormat. The expectation is that any diff represents a change in the tool which must be evaluated before updating the configuration file contents to match the new output. Although it seems some attempt was made at sorting the output in alphabetical order, it is not completely consistent. It turns out that arbitrary changes to the order of the keys may occur from one version to another, resulting in spurious diffs or difficult to read valid diffs. The solution is to sort the keys from the dumped output in a consistent order. This is done using the excellent yq tool. In addition to avoiding problematic diffs, this will make the configuration file easier to use.
During development of the ClangFormat configuration file some areas of the targeted test data C++ files where coverage could be increased were identified.
The test data that targets specific ClangFormat configuration keys was designed to provide coverage with the minimum amount of extraneous code required to make it valid C++. While experimenting with the IndentRequires configuration key introduced in ClangFormat 13, I discovered that, although valid code, ClangFormat does not correctly format the targeted test data for this key when the value is set to `true`. This could be considered a sign of quality of the test data having caught unwanted output that would result from a specific ClangFormat configuration. However, the problematic form of code, although it is appropriate for this unusual use case of test data, is unlikely to ever occur in a real program. For this reason, I think it is best to adjust the code slightly to a more realistic form that is also correctly handled by ClangFormat.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. Some of the configuration key type have been changed in the interim. These keys changed from a Boolean to an object data type, but an equivalent object value is available, which allows the value to be updated without any functional change to the output from the configuration.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. Configuration keys were added or expanded in the interim. Some of the keys allows defining patterns used for matching. Arduino's configuration does not have any need to define custom patterns. A base set of values is hard coded into ClangFormat, without any possibility of being overwritten. For the sake of transparency, these values are written into Arduino's configuration file
Some ClangFormat configuration keys have a base set of values which are hard coded into **ClangFormat**. These are appended to any custom values set in the configuration file and thus can not be overridden. Because this behavior of ClangFormat is unintuitive and not mentioned in the ClangFormat's documentation, notes were written for each such key. A significant number of additional keys were added in the interim between 11.0.1 and 14.0.0, which tipped the scales from documenting each key individually being reasonable. They are now listed under a dedicated section of the notes instead to improve the readability and maintainability of that content.
…ey for ClangFormat 14.0.0 The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. The values supported for the `AllowShortIfStatementsOnASingleLine` configuration key have changed in the interim, with the previous `Always` value no longer available. The equivalent value for ClangFormat 14.0.0 is `AllIfsAndElse`. Since the behavior resulting from this new value is not immediately apparent from the name or the official ClangFormat documentation, a note was added to document this. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. This was not possible to achieve using the configuration capabilities of ClangFormat 11.0.1, so the diff in the "golden master" test data is actually a reversion of unwanted formatting changes unavoidably produced by the previous version.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. The `false` value is the most permissive, allowing the user to break or not break at their whim. Since the concepts feature was added to C++ relatively recently, there is not any established style in official Arduino code.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. This was not possible to achieve using the configuration capabilities of ClangFormat 11.0.1, so the diff in the "golden master" test data is actually a reversion of unwanted formatting changes unavoidably produced by the previous version.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. This was not possible to achieve using the configuration capabilities of ClangFormat 11.0.1, so the diff in the "golden master" test data is actually a reversion of unwanted formatting changes unavoidably produced by the previous version.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The hope was that this new support for access modifier indentation would allow Arduino's ClangFormat configuration to achieve code formatting of classes matching the Arduino code style, and aligned with the behavior of the Arduino IDE 1.x "Auto Format" feature (which does indent them). Unfortunately, it turns out that the setting has as an undesirable side effect on struct formatting, which renders the indentation produced by setting it to `true` incompatible with the Arduino code style. For this reason, it was set to `false`, which results in no functional change to the existing configuration. A note has been added about this finding.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. Since the requires clauses feature was added to C++ relatively recently, there is not any established style in official Arduino code. The key setting was chosen based on a survey of the style in use in prominent reference code.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. Since we have `IndentPPDirectives` set to `None`, this option has no effect. It is set to `-1` to cause preprocessor directive indentation to be done according to `IndentWidth` for consistency in the case of custom user configurations which do enable `IndentPPDirectives`.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. This was not possible to achieve using the configuration capabilities of ClangFormat 11.0.1, so the diff in the "golden master" test data is actually a reversion of unwanted formatting changes unavoidably produced by the previous version.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. Since the `Penalty*` keys are not applicable when wrapping is disabled by setting `ColumnLimit: 0`, as is done in Arduino's configuration. These keys are all set to an arbitrary value of `1`.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. Since the `Penalty*` keys are not applicable when wrapping is disabled by setting `ColumnLimit: 0`, as is done in Arduino's configuration. They are all set to an arbitrary value of `1`.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. The `Leave` value is the most permissive.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Comply with the established Arduino code style The chosen `Pointer` value provides consistency with the pointer alignment style.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. - Comply with the established Arduino code style
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. The `Leave` value the most permissive.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. This key is only used when `FixNamespaceComments` is set to `true`. Since Arduino's configuration sets `FixNamespaceComments: false`, the value of `ShortNamespaceLines` doesn't have any direct effect, but the chosen value of `0` will provide the maximum possible level of consistency in custom user configurations that set `FixNamespaceComments: true`
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. Since this key is specific to the Java language, while Arduino's configuration is intended for use with Arduino language/C++/C, the value of this key is irrelevant so the default value is used. This fact was documented in the notes.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. - Comply with the established Arduino code style The key is set to `Default` in order to preserve the behavior of emulating the Arduino IDE 1.x "Auto Format" feature's permissive behavior by using whichever style is established in the code base (`DerivePointerAlignment: true`), or using the Arduino code style of right alignment (`PointerAlignment: Right`) when ClangFormat can not determine an alignment style from the code.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Comply with the established Arduino code style
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. - Comply with the established Arduino code style
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with behavior of the Arduino IDE 1.x "Auto Format" feature. This was not possible to achieve using the configuration capabilities of ClangFormat 11.0.1, so the diff in the "golden master" test data is actually a reversion of unwanted formatting changes unavoidably produced by the previous version.
The previous ClangFormat configuration was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by Arduino tools is 14.0.0. This configuration key was added in the interim. The key value set here was chosen in order to: - Align with the permissive behavior of the Arduino IDE 1.x "Auto Format" feature.
The ClangFormat configuration file was previously generated using ClangFormat 11.0.1. The output produced by that version did not contain a `BasedOnStyle` key, which is why it was not present in the configuration despite being supported by 11.0.1. Since Arduino's configuration sets all configuration keys, the `BasedOnStyle` key has no effect anyway. The configuration file generated using ClangFormat 14.0.0 does contain a `BasedOnStyle` key. Unfortunately, regardless of what was set in the configuration, the tool always outputs a key set to an empty string. Even though it is likely not an issue, this is concerning because it is not documented as a supported value.
In order to facilitate maintenance and development, the ClangFormat configuration file is standardized by running a `clang-format --dump-config` command while configured according to the current configuration file, then updating the file according to the dumped effective configuration. The configuration file generated using ClangFormat 14.0.0 contains a `BasedOnStyle` key set to an empty string (`BasedOnStyle: ''`), even when configured via the arbitrary `BasedOnStyle: LLVM` setting previously used by Arduino's configuration. Since Arduino's configuration sets all configuration keys, the `BasedOnStyle` key has no effect, and so this is likely not an issue. However, it is concerning because an empty string is not documented as a supported value. Out of an abundance of caution and to avoid confusion, the value of this key is reset to the documented valid value `LLVM`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ClangFormat configuration file hosted in this repository was developed for use with ClangFormat 11.0.1. The version of ClangFormat now in use by the Arduino IDE 2.x is 14.0.0:
https://github.com/arduino/arduino-ide/blob/19c0334a91cf562cf63edf0496eae16165499f8f/arduino-ide-extension/package.json#L164-L166
Configuration keys were added, removed, or changed in type in the interim.
The configuration file and its maintenance infrastructure are hereby updated for use with ClangFormat 14.0.0.
Functional changes
The goal will always be to preserve consistent formatter output through any changes that must be made to the file. Since Arduino IDE 2.x is still in a pre-release state, the output of most interest is that of the stable production Arduino IDE 1.x and the Arduino Web Editor, which use the Artistic Style code formatter.
The ClangFormat configuration file was designed to reproduce the results of the Artistic Style tool under the default configuration provided by the Arduino IDE 1.x installation. Unfortunately, it emerged at that time that some changes were unavoidable due to differences in the two tools, most often that ClangFormat tends to enforce whichever specific formatting style was chosen, without an option to simply accept whatever formatting the user happened to write, as was the case with Artistic Style.
Some of the newly added keys either offer a more lenient behavior or else allow specific behavior that aligns with the output from Artistic Style. This opportunity was taken to achieve a closer alignment of the outputs from the two tools. This resulted in some diffs in the "golden master" test data used to verify the output from ClangFormat is as expected under Arduino's configuration. However, these diffs are actually the previously unavoidable unwanted formatting differences being reverted.
Schema validation failure
The
validatejob of the "Check ClangFormat Configuration" workflow is still failing. This is caused by an error in the JSON schema, so it should not be considered an indication of a defect in this pull request or cause to delay its merge.I have submitted a fix for that error and the job will pass once that is accepted:
SchemaStore/schemastore#2384