attribute-ordering: Add attribute ordering check

Based on https://discourse.nixos.org/t/document-attribute-ordering-in-package-expressions/4887.

explanations/attribute-ordering.md

@@ -0,0 +1,117 @@
+Many nixpkgs maintainers like having consistent ordering so we can quickly locate things we need to change. This is nothing but a personal preference but [growing stronger](https://discourse.nixos.org/t/document-attribute-ordering-in-package-expressions/4887).
+
+We start with general package information and sources that change every update, then list dependencies, set up build, potentially override the default builder phases and finally provide metadata. This is sort of high level to low level ordering but not exactly.
+
+## Examples
+```nix
+{ stdenv 
+, fetchurl
+}:
+
+stdenv.mkDerivation rec {
+  pname = "";
+  version = "";
+
+  # Outputs are also part of store paths.
+  outputs = [ ];
+
+  src = fetchurl {
+    url = "";
+    sha256 = "";
+  };
+
+  # Patches often depend on source code and need to be kept in sync.
+  patches = [
+  ];
+
+  # Dependencies. Build-time dependencies (native) first, then regular, then propagated variants of the two, then dependencies for tests.
+  nativeBuildInputs = [
+  ];
+
+  buildInputs = [ 
+  ];
+
+  propagatedNativeBuildInputs = [
+  ];
+
+  propagatedBuildInputs = [
+  ];
+
+  checkInputs = [
+  ];
+
+  installCheckInputs = [
+  ];
+
+  # Build systemd configuration flags.
+  configureFlags = [
+  ];
+  # or cmakeFlags, mesonFlags
+  
+  # Build tool flags common for all phases.
+  makeFlags = [
+  ];
+  # or ninjaFlags
+
+  # 
+  buildFlags = [
+  ];
+
+  installFlags = [
+  ];
+
+  # Environment variables.
+  FONTCONFIG_FILE = "";
+  NIX_CFLAGS_COMPILE = "";
+
+  # Enabling off-by-default phases.
+  doCheck = true;
+  doInstallCheck = true;
+  
+  # Disabling on-by-default phases.
+  dontBuild = true;
+
+  # Attributes for overriding default phases, and their hooks should be ordered exactly as they are executed in setup.sh (https://github.com/NixOS/nixpkgs/blob/18f47ecbac1066b388e11dfa12617b557abeaf66/pkgs/stdenv/generic/setup.sh#L1261).
+  # preUnpack
+  # unpackPhase
+  # postUnpack
+  # prePatch
+  # patchPhase
+  # postPatch
+  # preConfigure
+  # configurePhase
+  # postConfigure
+  # preBuild
+  # buildPhase
+  # postBuild
+  # preCheck
+  # checkPhase
+  # postCheck
+  # preInstall
+  # installPhase
+  # postInstall
+  # preFixup
+  # fixupOutput
+  # fixupPhase
+  # postFixup
+  # preInstallCheck
+  # installCheckPhase
+  # postInstallCheck
+
+  # Metadata
+  passthru = {
+  };
+
+  meta = with stdenv.lib; {
+    description = "";
+    longDescription = ''
+    '';
+    homepage = "";
+    license = licenses.mit;
+    platforms = platforms.unix;
+    maintainers = with maintainers; [
+      somebody
+    ];
+  };
+}
+```

lib/derivation-attributes.nix

@@ -0,0 +1,126 @@
+[
+  "name"
+  "pname"
+  "version"
+
+  "outputs"
+
+  "src"
+  "srcs"
+  "patches"
+
+  {
+    name = "lists of dependencies";
+    values = [
+      "nativeBuildInputs"
+      "buildInputs"
+      "propagatedNativeBuildInputs"
+      "propagatedBuildInputs"
+      "checkInputs"
+      "installCheckInputs"
+    ];
+  }
+
+  {
+    name = "build systemd configuration flags";
+    values = [
+      "cmakeFlags"
+      "mesonFlags"
+      "configureFlags"
+      "qmakeFlags"
+    ];
+  }
+
+  {
+    name = "build tool flags";
+    values = [
+      "makeFlags"
+      "ninjaFlags"
+      "buildFlags"
+      "checkTarget"
+      "checkFlags"
+      "installFlags"
+      "installCheckFlags"
+      "enableParallelBuilding"
+      "enableParallelChecking"
+    ];
+  }
+
+  {
+    name = "environment variables";
+    values = [
+      "CFLAGS"
+      "CXXFLAGS"
+      "FONTCONFIG_FILE"
+      "NIX_CFLAGS_COMPILE"
+      "NIX_CFLAGS_LINK"
+      "NIX_ENFORCE_NO_NATIVE"
+      "NIX_LDFLAGS"
+    ];
+  }
+
+  {
+    name = "attributes for enabling off-by-default phases";
+    values = [
+      "doCheck"
+      "doInstallCheck"
+      "doDist"
+    ];
+  }
+
+  {
+    name = "attributes for disabling on-by-default phases";
+    values = [
+      "dontUnpack"
+      "dontPatch"
+      "dontConfigure"
+      "dontBuild"
+      "dontUseNinjaBuild"
+      "dontInstall"
+      "dontUseNinjaInstall"
+      "dontFixup"
+      "dontUseNinjaCheck"
+    ];
+  }
+
+  {
+    name = "attributes for overriding default phases";
+    values = [
+      "preUnpack"
+      "unpackPhase"
+      "postUnpack"
+      "prePatch"
+      "patchPhase"
+      "postPatch"
+      # "preConfigurePhases"
+      "preConfigure"
+      "configurePhase"
+      "postConfigure"
+      # "preBuildPhases"
+      "preBuild"
+      "buildPhase"
+      "postBuild"
+      "preCheck"
+      "checkPhase"
+      "postCheck"
+      # "preInstallPhases"
+      "preInstall"
+      "installPhase"
+      "postInstall"
+      # "preFixupPhases"
+      "preFixup"
+      "fixupOutput"
+      "fixupPhase"
+      "postFixup"
+      "preInstallCheck"
+      "installCheckPhase"
+      "postInstallCheck"
+      # "preDistPhases"
+      "distPhase"
+      # "postPhases"
+    ];
+  }
+
+  "passthru"
+  "meta"
+]

overlays/attribute-ordering.nix

@@ -0,0 +1,55 @@
+{ builtAttrs
+, packageSet
+, namePositions
+}@attrs:
+
+final: prev:
+let
+  inherit (prev) lib;
+  inherit (import ../lib { inherit lib; }) checkMkDerivationFor;
+
+  preferredOrdering =
+    let
+      flattenGroup = item:
+        if builtins.isAttrs item then
+          map (subitem: { group = item.name; attr = subitem; }) item.values
+        else
+          [ { attr = item; } ];
+      addOrder = n: i: i // { order = n; };
+    in
+      lib.pipe (import ../lib/derivation-attributes.nix) [
+        (builtins.concatMap flattenGroup)
+        (lib.imap0 addOrder)
+        (builtins.map ({ attr, order, group ? null }@attrs: {
+          name = attr;
+          value = { inherit order group; };
+        }))
+        builtins.listToAttrs
+      ];
+
+  checkDerivation = drv:
+    let
+      getAttrLine = attr: (builtins.unsafeGetAttrPos attr drv).line;
+
+      drvAttrs = builtins.sort (a: b: getAttrLine a < getAttrLine b) (builtins.attrNames drv);
+
+      knownDrvAttrs = builtins.filter (attr: preferredOrdering ? "${attr}") drvAttrs;
+
+      sideBySide = lib.zipLists knownDrvAttrs (builtins.tail knownDrvAttrs);
+    in
+      lib.forEach sideBySide ({ fst, snd }:
+        let
+          fstInfo = preferredOrdering.${fst};
+          sndInfo = preferredOrdering.${snd};
+        in {
+          cond = fstInfo.order > sndInfo.order;
+          msg = ''
+            The ${lib.optionalString (fstInfo.group != null) "${fstInfo.group}, including the "}attribute “${fst}” should preferably come before ${lib.optionalString (sndInfo.group != null) "${sndInfo.group}’ "}“${snd}” attribute in the expression.
+
+            See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/attribute-ordering.md
+          '';
+        }
+      );
+
+in
+  checkMkDerivationFor attrs prev checkDerivation

run-tests.py

@@ -60,6 +60,16 @@ class TestSuite(unittest.TestSuite):
     _cleanup = False
 
     def __iter__(self):
+        yield make_test_rule(
+            'attribute-ordering',
+            [
+                'out-of-order',
+            ],
+            [
+                'properly-ordered',
+            ]
+        )
+
         yield make_test_rule(
             'build-tools-in-build-inputs',
             [

tests/attribute-ordering/default.nix

@@ -0,0 +1,10 @@
+{ pkgs
+}:
+
+{
+  # positive cases
+  out-of-order = pkgs.callPackage ./out-of-order.nix { };
+
+  # negative cases
+  properly-ordered = pkgs.callPackage ./properly-ordered.nix { };
+}

tests/attribute-ordering/out-of-order.nix

@@ -0,0 +1,24 @@
+{ stdenv 
+, fetchurl
+}:
+
+stdenv.mkDerivation rec {
+  meta = with stdenv.lib; {
+    description = "";
+    license = licenses.mit;
+    platforms = platforms.unix;
+    maintainers = [];
+  };
+
+  passthru = { };
+
+  nativeBuildInputs = [];
+
+  patches = [];
+
+  src = ../fixtures/make;
+
+  version = "0.0.0";
+
+  pname = "out-of-order";
+}