Configuration file

[ematipico]

Rome will have to support a configuration file, where the user can store their preferences and rome will pick it up and apply all the information to the tools.

Here's a link to the configuration of the rome classic:

https://github.com/rome/tools/blob/archived-js/internal/project/types.ts#L58-L104

Warning not all the fields are currently used. Many of them were prepared for future developments. For example typeCheck is not used, but it will be used by the type checker feature. Another example is cache.

This the "default" config file, where Rome classic provides its defaults:

https://github.com/rome/tools/blob/archived-js/internal/project/types.ts#L251-L320

Below, there's a thread for each big section that we will have inside the configuration file.

Note all tools are Enableable, meaning that support a enabled: true

[ematipico]

config.formatter

This is the section related to the formatter. Here's my suggestion of how section should be:

{
	"formatter": {
		"enabled": true,
		"indentStyle": "space",
		"spaceNumber": 4, 
		"lineWidth": 120,
		"js": {
			"quote_style": "double"
		},
		"ignore": [ "./path", "./path/to/file.js" ]
	}
}

The reason why I moved "quote_tyle" under a "js" section is because the quoting style is not something that can be applied to all languages. Some languages don't have the luxury to choose a different quote style.

Inside "js" we could have all the options that we will port from Prettier.

"ignore" will contain a list of "patterns" where we specify files to ignore.

I am not sure about the shape of "indentStyle", because in case of spaces, we have to provide the number of spaces. Another alternative is the following:

{
	"formatter": { 
		"indentStyle": {
			"type": "space",
			"quantity": 4
		}
	}
}

"quantity" is optional. Ignored in case indentStyle is "tab", read in case we have spaces and it's optional. We will fall back to our default in case it's not there.

[Conaclos]

spaceNumber and tabWidth could be merged into indentSize.

This is the approach used by .editorconfig. EditorConfig supports also tabWidth (it defaults to indentSize when unspecified).

{
	"formatter": {
		"enabled": true,
		"indentStyle": "space",
		"indentSize": 4
	}
}

{
	"formatter": {
		"enabled": true,
		"indentStyle": "tab",
		"indentSize": 8
	}
}

[ematipico]

config.lint

This is the section related to the linter. Here's my suggestion of the config could be:

{
	"lint": {
		"enabled": true,
		"globals": ["$"],
		"ignore": [""],
		"requireSuppressionExplanations": true,
		"disabledRules": {
			"js": [
				"noVar"
			],
			"jsx": [
				"noPropSpreading"
			]
		}
	}
}

• globals are global objects that should be "taken for granted", for example $ in case jQuery is loaded from a another script;
• disabledRules are split by category;

In theory we should be able to codegen the struct that will support disabledRules, and automatically generated by reading the lint rules that we have in the source code.

[ematipico]

In rome classic, we hadn't had a case where a global was valid in file but invalid in another. If it's not that common, we could hold it for now and just go with a global globals

[ematipico]

this was what Jamie suggested time ago: #1529 (comment)

[ematipico]

This is a follow up from #2890 (review)

Interestingly, this raises the question of whether generating a static struct for the rules configuration is the best solution, compared for instance to deserializing the rules field to a HashMap<String, Box<RawValue>> and letting the analyzer handle the deserialization of the inner data (the main advantage I see to doing that is the ability for the analyzer to emit a custom diagnostic for invalid configurations, eg. the rule "noDeadCode" does not exist, did you mean "js/noDeadCode" ?)

It really depends on how we want to play it.

Having the configuration shape defined at the beginning would allow us to:

• generate documentation of the configuration for the website
• know the shape of the configuration early, and catch possible errors early (for example misspelled lint rules)
• the validation occurs regardless, even when linter is "enabled": false

This will make more difficult to:

• refine possible configuration errors, like you suggested
• ...anything else?

Doing this inside the analyzer allow us to:

• allow to us for a better control of what comes or out
• the "validation" happens only when running the linter, only when linter is "enabled": true

This will make more difficult to:

• know the shape of configuration ahead of time
• generate documentation

[MichaReiser]

My understanding of the recommendation is to have a custom deserialisation implementation that has more clever error recovery than what serde offers out of the box.

The code gen would generate the necessary metadata (or we use our own #[deserialize] macro on the rule) to know which fields a rule supports, especially when it comes to additional options.

[ematipico]

Yes, I believe this is doable using a custom deserialisation method.

[ematipico]

config.parser

Configuration related to our internal parsers. Here's my suggestions:

{
	"parser": {
		"jsxEverywhere": true
	}
}

• jsxEverywhere tells the parser to enable JSX inside .js files. By default, false.

Not sure if, for now, we could have more configuration options for the parser.

[MichaReiser]

The parser options should probably be language-specific.

[ematipico]

Makes sense

[ematipico]

config.dependencies

This configuration tells rome if it should check dependencies too. This is not a signal to Rome to build the dependency graph.

{
	"dependencies": {
		"enabled": true,
		"invalidLicenses": ""
	}
}

Enabling this tools will tell to rome to check if a dependency actually exists, run some health check on their manifests, and emit a diagnostic during rome check.

Let's suppose we have

import { ReactDOM } from "react-dom";

But it's not installed, then Rome will emit a diagnostic.

It will emit a diagnostic if the dependency has an invalid license.

[leops]

Currently the configuration schema I've implemented for the language-specific settings in rome_service is something like:

{
    "formatter": {
        "enabled": true,
        "indentStyle": "space",
        "spaceNumber": 4,
        "lineWidth": 120
    },
    "linter": {
        "enabled": true
    },
    "languages": {
        "javascript": {
            "formatter": {
                "indentStyle": "tabs",
                "quoteStyle": "double"
            }
        },
        "css": {
            "linter": {
                "enabled": false
            }
        }
    }
}

The main difference with the proposed config.formatter is that it has a single languages map with one section for each language and sub-sections for each tool, instead of having each tool-specific section have additional language-specific sub-sections.

Additionally in this schema each language can specify not just their specific options but also override the entire set of supported global settings, for instance to only enable the linter or formatter for specific languages. I'm not entirely certain this is something we actually want to support though, or if it fits with the "configuration philosophy" of the project

[ematipico]

This is something that we haven't actually managed to achieve, but I had started some works around it. There was a proposal from Jamie (which I can't find anywhere unfortunately) where we were re writing how the linter configuration was, and it was roughly the way I presented here.

Regarding the philosophy, what we really want to break is the fact of having less configuration as possible. Less maintenance for us and less time spent by the users to understand and apply the configuration.

For example, about the formatter, It think it's preferable to have a global in "indentStyle" for all the files, unless a particular file doesn't support that option (YAML, for example, doesn't spaces). I don't see any particular value in giving the possibility to change this option for each language.

[NicholasLYang]

Should we consider putting a version number in the config to allow for future breaking changes?

[ematipico]

Could you give us an example of how this version number work and it would prevent "breaking change"? We already have the version number in the package.json

[NicholasLYang]

It's standard in formats such as WebAssembly to start with a version number, so that if we introduce a breaking change to the format, we can increment the version to help with parsing. That way a parser can see that it's parsing a rome.json v2 and not a rome.json v1 and either parse it differently, or gracefully fail.

While we could use the package.json version, that would require reading the package.json. Not impossible ofc, but perhaps less preferable to just having a version in the config itself.

It's not super essential but it can potentially solve issues in the future to include a version number now.

[leops]

In general including a version number from the start is only required if the serialization / disk format might change. Since we're using JSON we don't really have this problem though, and if we ever need to introduce breaking changes to the configuration schema we could then introduce a new "version": "2" field and consider the default / missing value to be "1"

[ematipico]

On that regard, the latest nightly of Rome classic shipped with a command called rome migrate, which was in charge of migrating configuration breaking changes from a version to another, using mutation APIs on the AST of the configuration file.

Each migration was tagged with a version number and run in order using semver.

[decaffjoe]

Should/can support for the --write CLI option and target filepath be added to rome.json?

In general, I think it would be nice to have all of the CLI flags supported in rome.json. I would like to have all the settings in one place and preferably for that one place to be rome.json.

Right now you either have to full commit to CLI flags in package.json or split up the configuration to achieve the below setup:

// package.json

scripts: {
    "format": "rome format --write ./src"
}

// rome.json

{
    "formatter": {
        "indentStyle": "space"
    }
}

Ideal world:

// package.json

scripts: {
    "format": "rome format"
}

// rome.json

{
    "formatter": {
        "indentStyle": "space",
        "write": true,
        "filepath": "./src"
    }
}

[ematipico]

Thank you for pitching in, that's and interesting idea!

I thought about it, and this set up would cause some issue with how the CLI works nowadays. For example, with the ideal scenario you mentioned, any format command would write to file system and there wouldn't a way to run the command without writing to file system (and the other way around).

[decaffjoe]

Ah yes, I see your point- however, I think this would be desired behavior in my use case. For context, I am approaching this with the understanding that I will have one rome.json file for each different project e.g.

all_my_repos
├── personal_web_project
│   |── package.json
│   └── rome.json
└── react_work_project
    |── package.json
    └── rome.json

If this is the correct way to use rome.json files, then when I'm in the personal_web_project directory I want the npm run format command to always do the same thing.

Are you saying that if I'm editing personal_web_project/src/someFile.js and just want rome to format it but not save it, it's impossible since running rome format will always pick up the rome.json configuration file in personal_web_project and enforce the "write": true option?

If so, maybe the CLI could have a -c or --config-file flag that would tell it to look for a rome.json file. If the -c flag is not used then the CLI will not use rome.json. With this setup you could have:

// package.json

scripts: {
    "format": "rome format --config-file",
    "format_cli": "rome format"
}

// rome.json

{
    "formatter": {
        "indentStyle": "space",
        "write": true,
        "filepath": "./src"
    }
}

Here you could run npm run format to get the behavior you want 99% of the time, but if you need to run rome format with different options you could run npm run format_cli --whichever-flags --we-want-to-use

Maybe this is just too much complexity though. Also I could easily be misunderstanding the context of where and when rome.json is used.

[leops]

Ultimately while we would very much like to have the CLI flags in sync with the configuration file format, I think the --write would explicitly be an exception to that for two reasons: one is that it's a CLI-only option that doesn't mean anything for the editor extension for instance, but the second and most important issue is that we consider it to be a "dangerous" option since it overwrites files on the disk and may cause data loss, and as such we always want users to explicitly write it down in the command line (this aligns with the equivalent option in Prettier, which doesn't allow specifying "write": true in .prettierrc.json either)