Skip to content
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

Host standardized ClangFormat configuration file #250

Merged
merged 14 commits into from
Jul 26, 2022
Merged

Host standardized ClangFormat configuration file #250

merged 14 commits into from
Jul 26, 2022

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Jul 26, 2022

ClangFormat is now the preferred code formatting tool for use in tooling and Arduino language/C++/C firmware projects.

An Arduino code style has been established by the Arduino IDE's "Auto Format" feature and the code of the sketches and libraries written by Arduino. It is important that this style remain consistent even through this time of transition from the former preferred Artistic Style code formatter tool. A ClangFormat configuration was carefully developed to achieve this goal.

This complex configuration must be properly maintained through updates of the actively developed LLVM/clang/ClangFormat project. This can only be achieved if there is a single canonical source of the file, from which any enhancements and fixes are pulled from and pushed to by the various projects which use it, such as:

This repository will now be used to host and maintain that canonical upstream copy.

The scope of this PR is the following:

Changes without any functional effect have been made to the configuration file to assist with its maintenance:

  • Move notes from comments in the configuration file to a dedicated document
  • Standardize the file format to match that used by ClangFormat

The added infrastructure consist of:

  • Validation against the JSON schema
  • Script for conversion to the data format used by Arduino IDE 2.x
  • Comparison of ClangFormat output against an extensive set of "golden master" Arduino language/C++/C code
  • Comparison of configuration file against the effective configuration of ClangFormat when used with that file
  • GitHub Actions workflow that runs each of the above automatically on every push or PR that modifies relevant files, periodically, and when manually triggered (with the option of specifying an arbitrary ClangFormat version to be used).

See the individual commit messages for more details on each

Fixes arduino/arduino-ide#680

ClangFormat is now the preferred code formatting tool for use in tooling and Arduino language/C++/C firmware projects.

An Arduino code style has been established by the Arduino IDE's "**Auto Format**" feature and the code of the sketches
and libraries written by Arduino. It is important that this style remain consistent even through this time of transition
from the former preferred Artistic Style code formatter tool. A ClangFormat configuration was carefully developed to
achieve this goal.

This complex configuration must be properly maintained through updates of the actively developed LLVM/clang/ClangFormat
project. This can only be achieved if there is a single canonical source of the file, from which any enhancements and
fixes are pulled from and pushed to by the various projects which use it, such as:

- Arduino IDE 2.x
- Arduino Language Server
- Firmware projects (via the "Check C++" workflow template)
- Arduino Cloud (pending)

This repository will now be used to host and maintain that canonical upstream copy.

This is the unmodified configuration which is already in use by Arduino IDE 2.x and Arduino Language Server.

The YAML format source:

https://github.com/arduino/arduino-language-server/blob/0.7.1/ls/ls_formatter.go#L15-L160

Which is functionally equivalent to the JavaScript object generated from it now in use by Arduino IDE 2.x:

https://github.com/arduino/arduino-ide/blob/2.0.0-rc9/arduino-ide-extension/src/node/clang-formatter.ts#L133-L278
A `clang-format:validate` task is added to the repository infrastructure. In addition to the option to run this task
locally during maintenance on the file, it is executed automatically by the repository's CI system on every change to
relevant files and periodically to catch problems exposed by external changes (e.g., schema updates).
An ESLint configuration file provides the standardized configuration for use in Arduino tooling projects. This uses the
Airbnb base configuration. Since Prettier provides code formatting, overlapping ESLint rules are disable via
eslint-config-prettier.

A task is provided to run ESLint on the JavaScript code files of the project the assets are applied to.

On every push and pull request that affects relevant files, and periodically, a GitHub Actions workflow runs this task.
An `eslint:validate` task is added to the repository infrastructure.

In addition to the option to run this task locally during maintenance on the file, it is executed automatically by the
repository's CI system on every change to relevant files and periodically to catch problems exposed by external changes
(e.g., schema updates).
…no IDE 2.x

The Arduino IDE 2.x "Auto Format" feature uses ClangFormat to format the sketch file open in the editor.

The default ClangFormat configuration is embedded in object form in the Arduino IDE code base, not in the YAML format
used by `clang-format`.

A script makes the required conversion.

The `clang-format:convert` task executes the script.

In order to facilitate updates, A GitHub Actions workflow runs the conversion on every change to relevant files and
makes it available for download as a workflow artifact.
Two tasks are added:

`js:fix`: Automatically correct ESLint rule violations when possible.
`js:lint`: Check for ESLint rule violations.

On every push and pull request that affects relevant files, and periodically, a GitHub Actions workflow runs ESLint on
the repository's JavaScript files.

ESLint is configured via the .eslintrc.yml file:
https://eslint.org/docs/user-guide/configuring/configuration-files
These are sketches from official Arduino boards platforms, libraries, and standalone sketches.

This data will be used to evaluate the impact of any proposed changes to the formatter configuration on the code in use
by the Arduino commmunity.
…uino code style

The base level Arduino code style is provided by the default output of the Arduino IDE 1.x "Auto Format" feature.

The Arduino IDE 2.x Auto Format feature must be configured to reproduce that style as closely as possible. For this
reason, the "golden master" test data is formatted to that style. This will be compared against the output generated by
Arduino's ClangFormat configuration.

The official Arduino sketches used as "samples" test data was imported into the repository exactly as it exists in the
upstream projects. Unfortunately, some of that code does not comply with the base Arduino code style, so it must be
brought into compliance by formatting using the 2.05.1 version of the Artistic Style code formatting tool used by Arduino IDE 1.8.19:

https://github.com/arduino/Arduino/blob/1.8.19/build/build.xml#L483

with the default formatter configuration of Arduino IDE 1.8.19:

https://github.com/arduino/Arduino/blob/1.8.19/build/shared/lib/formatter.conf
The ClangFormat configuration file hosted in this repository was developed for ClangFormat 11.0.1, the version in use by
Arduino Language Server and Arduino IDE 2.x at that time.

Significant effort was made to create a configuration that would result in the same output as that of the Arduino IDE
1.x "Auto Format" feature. Where differences between ClangFormat and the Artistic Style formatter tool of Arduino IDE
1.x made that impossible, the configuration was based on surveys of existing official Arduino code.

This commit demonstrates the unavoidable changes that resulted from the configuration tool change.
These files specifically target each relevant configuration setting of ClangFormat 14.0.0, the version currently in use
by Arduino IDE 2.x and Arduino Language Server.

In combination with the real world Arduino sketch samples, this should provide full coverage of the result of proposed
changes to the configuration file or updates to the ClangFormat version in use.

The "golden masters" were formatted using ClangFormat 11.0.1, which is the version the configuration file was developed
for.
ClangFormat is a very complex tool under active development. This means that unintended changes to the formatter output
might result from:

- Unintended side effects of intentional configuration changes.
- Differences in ClangFormat's handling of existing configuration options after updating to a new version.
- Introduction of new configuration options, which will be set to the LLVM style default value if not set in Arduino's
  configuration, after updating to a new version.

For this reason, it is necessary to carefully validate the configuration in preparation for:

- Changes to the configuration file
- Updating the version of ClangFormat used in Arduino's projects

Validation is done by formatting a collection of test data files and then comparing the result against "golden masters"
which have the intended formatting. The repository's continuous integration system does this check after every change to
relevant files. It can also be run locally using the `clang-format:check-configuration` task.

Arbitrary ClangFormat versions may be specified for use in the check in order to evaluate a version of ClangFormat under
evaluation for an update candidate.
The ClangFormat configuration file hosted in this repository is used by several projects.

Previously, comments in the configuration file were used to record some potentially useful information about some of the
configuration options. This non-functional content increases the burden of maintaining the downstream copies. A more
efficient approach is to have a single copy of the information stored here, easily accessible from the downstreams via
the source link.
The LLVM/Clang/ClangFormat project is under very active development, with configuration keys regularly being added,
changed to a different data type, or removed.

When a key is set in the configuration file, the default LLVM style value is used. This means that updating to a new
version of ClangFormat might result in unintended changes to the results from the configuration file. In the case of a
change of type, it seems the ClangFormat developers generally will provide backwards compatibility of some sort, but
there is no guarantee that the resulting configuration will be exactly the same as was intended when the value of the
key was selected for use in Arduino's configuration file.

ClangFormat has a useful `--dump-config` flag which outputs the effective configuration. This allows us to determine
whether there are any differences between the intended configuration as defined by Arduino's configuration file and the
actual configuration of the tool.

In order to accomplish this, it is best to use the exact format output by `clang-format --dump-config`, which allows any
differences to be detected and seen clearly using a simple diff command.

The only exception is the insertion of a comment providing the source URL of the configuration file in order to
facilitate the syncing of changes with the downstream copies of the file in the various projects that use it.
ClangFormat has an extensive and complex configuration system. Although the documentation is excellent and
comprehensive, some additional knowledge was hard earned during the development of the Arduino ClangFormat
configuration.

Where appropriate, some of this information was recorded in comments in the original configuration file, which have
already been transferred to the notes document. However, that original comment format was limiting so some of the
information was not previously recorded there.

The additional information is hereby added to the notes document.
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jul 26, 2022
@per1234 per1234 self-assigned this Jul 26, 2022
@per1234
Copy link
Contributor Author

per1234 commented Jul 26, 2022

The configuration was developed and validated for use with ClangFormat 11.0.1, the version of the tool in use by Arduino Language Server and Arduino IDE 2.x at that time. For that reason, all golden masters added by this PR were generated using ClangFormat 11.0.1.

Since the time the configuration file was developed, there have been several updates of the ClangFormat in use in Arduino IDE 2.x/Arduino Language Server (without any accompanying updates or validation of the configuration file). The infrastructure is configured to validate the configuration file against the ClangFormat version it is actually in use with currently by Arduino IDE 2.x: 14.0.0.

This version mismatch is the reason for the failing CI jobs of the "Check ClangFormat Configuration" workflow run:

https://github.com/arduino/tooling-project-assets/actions/runs/2737726990

  • Validation against JSON schema is failing due to changes to key data types since 11.0.1
  • Configuration data check is failing due to additions, removals, and type changes of keys since 11.0.1
  • Output check is failing due to a change in the behavior of ClangFormat since 11.0.1

These will be resolved by updating the configuration file and the test data for ClangFormat 14.0.0. That work is out of scope for this PR. I will submit those changes via a separate PR.

Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing job Per. As always

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for changes to formatter output resulting from clangd bump
2 participants